Transactions cleanup #817
Transactions cleanup #817
Conversation
…ction-sidebar-partials-and-controller-cleanup
This reverts commit 9cc34e3.
…ction-sidebar-partials-and-controller-cleanup # Conflicts: # app/controllers/transactions_controller.rb # app/models/transaction.rb
This reverts commit 7f7a289.
| @@ -2,7 +2,7 @@ | |||
| <% is_selected = category.id === @selected_category&.id %> | |||
|
|
|||
| <%= content_tag :div, class: ["filterable-item flex justify-between items-center border-none rounded-lg px-2 py-1 group w-full", { "bg-gray-25": is_selected }], data: { filter_name: category.name } do %> | |||
| <%= button_to transaction_path(@transaction, transaction: { category_id: category.id }), method: :patch, class: "flex w-full items-center gap-1.5 cursor-pointer" do %> | |||
| <%= button_to transaction_row_path(@transaction, transaction: { category_id: category.id }), method: :patch, data: { turbo_frame: dom_id(@transaction) }, class: "flex w-full items-center gap-1.5 cursor-pointer" do %> | |||
There was a problem hiding this comment.
There may be a better way to do this. Since this is a nested frame, I decided to target the outermost "row" frame so when the user selects a category, it replaces the entire row. We could certainly get more surgical with this (via streams), but my intention was to move towards the simplest solution possible, hence the Transactions::RowsController in tandem with a turbo_frame_tag dom_id(@transaction)
| label: t(".select_tags"), | ||
| class: "placeholder:text-gray-500" | ||
| }, | ||
| "data-auto-submit-form-target": "auto" %> |
There was a problem hiding this comment.
This is an interim solution since we don't yet have a design system for custom select dropdowns. When we get Lookbook going along with ViewComponent, we'll eventually replace this multi-select with something a bit easier for the user. In the meantime, I opted for a completely html-native solution to avoid having to maintain a partially built multi-select flow.
.github/workflows/ci.yml
Outdated
|
|
||
| - name: System tests | ||
| run: DISABLE_PARALLELIZATION=true bin/rails test:system | ||
| continue-on-error: true # TODO: Eventually we'll enforce for PRs |
There was a problem hiding this comment.
See issue description for explanation of this.
…ction-sidebar-partials-and-controller-cleanup # Conflicts: # .github/workflows/ci.yml
zachgoll
deleted the
795-transaction-sidebar-partials-and-controller-cleanup
branch
A complete overhaul and cleanup of the transactions feature in preparation for adding transaction types, rules, bulk editing, and more.
Overview
We've done quite a bit of work and discovery on the transactions feature of the app and due to the speed we have been adding new features, it had started to get a bit messy. The goal of this PR was to move the codebase to a better spot so that new transaction features like "Rules" and "Linked transactions" could be implemented easier.
Below is a summary of changes. I have also added a few comments throughout the PR for reference:
Search endpoint
The search endpoint has been drastically simplified to optimize for readability and extendability over all other factors.
POSTrequest + sessions to a simpleGETrequest with query paramsPartials + Controller + Turbo frames cleanup
We're rendering transaction rows in several places across the app:
All of these have slightly different requirements for rendering transactions, and previously, we were trying to share them, which broke encapsulation and added complexity in a lot of ways. To help with this, I've created context-specific transaction partials and used a composition approach to build the needed row for each scenario.
Furthermore, I've added a
Transactions::RowsControllerwhich is solely responsible for handling "inline" updates to a transaction row. This allows us to assign the "base case" to theTransactionsControllerand rendershowinto the "drawer" after updates and handle all of the row updates via turbo frames via theTransactions::RowsController.Transaction sync trigger logic
There's still probably a bit of work to do here, but I've consolidated all of the logic for calculating the start date for an account sync into the
Transactionmodel so that@transaction.sync_account_latercan be called from the controller."Drawer" vs. "Modal" distinction
To better distinguish between UI elements and their semantic meaning, I've renamed "Sidebar Modal" to "Drawer".
Transaction creation happens in a "modal" and show/edit happens in a "drawer". This separation also makes it a lot clearer when opening turbo frames.
System tests
Prior to this refactor, I added several integration and system tests to make sure nothing major broke during the refactor.
Given how flaky system tests can be, we're still a bit early to be enforcing them on PRs. That said, it's nice to have them running in CI for PR reviews and to get a feel for their stability, so I've added a system test step that will run but not enforce the tests.
Unit / integration tests are still enforced.
Other notes
There is a lot here and likely a few regressions that I've missed along the way that our test suite didn't catch. I'll be following up with additional PRs and added tests as we discover them.