mirror of
https://github.com/maybe-finance/maybe.git
synced 2025-07-18 20:59:39 +02:00
perf(transactions): add kind to Transaction model and remove expensive Transfer joins in aggregations (#2388)
* add kind to transaction model * Basic transfer creator * Fix method naming conflict * Creator form pattern * Remove stale methods * Tweak migration * Remove BaseQuery, write entire query in each class for clarity * Query optimizations * Remove unused exchange rate query lines * Remove temporary cache-warming strategy * Fix test * Update transaction search * Decouple transactions endpoint from IncomeStatement * Clean up transactions controller * Update cursor rules * Cleanup comments, logic in search * Fix totals logic on transactions view * Fix pagination * Optimize search totals query * Default to last 30 days on transactions page if no filters * Decouple transactions list from transfer details * Revert transfer route * Migration reset * Bundle update * Fix matching logic, tests * Remove unused code
This commit is contained in:
Zach Gollwitzer
GitHub
parent
7aca5a2277
commit
1aae00f586
49 changed files with 1749 additions and 705 deletions
|
|
@ -66,54 +66,7 @@ All code should maximize readability and simplicity.
|
|||
- Example 1: be mindful of loading large data payloads in global layouts
|
||||
- Example 2: Avoid N+1 queries
|
||||
|
||||
### Convention 5: Use Minitest + Fixtures for testing, minimize fixtures
|
||||
|
||||
Due to the open-source nature of this project, we have chosen Minitest + Fixtures for testing to maximize familiarity and predictability.
|
||||
|
||||
- Always use Minitest and fixtures for testing.
|
||||
- Keep fixtures to a minimum. Most models should have 2-3 fixtures maximum that represent the "base cases" for that model. "Edge cases" should be created on the fly, within the context of the test which it is needed.
|
||||
- For tests that require a large number of fixture records to be created, use Rails helpers such as [entries_test_helper.rb](mdc:test/support/entries_test_helper.rb) to act as a "factory" for creating these. For a great example of this, check out [forward_calculator_test.rb](mdc:test/models/account/balance/forward_calculator_test.rb)
|
||||
- Take a minimal approach to testing—only test the absolutely critical code paths that will significantly increase developer confidence
|
||||
|
||||
#### Convention 5a: Write minimal, effective tests
|
||||
|
||||
- Use system tests sparingly as they increase the time to complete the test suite
|
||||
- Only write tests for critical and important code paths
|
||||
- Write tests as you go, when required
|
||||
- Take a practical approach to testing. Tests are effective when their presence _significantly increases confidence in the codebase_.
|
||||
|
||||
Below are examples of necessary vs. unnecessary tests:
|
||||
|
||||
```rb
|
||||
# GOOD!!
|
||||
# Necessary test - in this case, we're testing critical domain business logic
|
||||
test "syncs balances" do
|
||||
Holding::Syncer.any_instance.expects(:sync_holdings).returns([]).once
|
||||
|
||||
@account.expects(:start_date).returns(2.days.ago.to_date)
|
||||
|
||||
Balance::ForwardCalculator.any_instance.expects(:calculate).returns(
|
||||
[
|
||||
Balance.new(date: 1.day.ago.to_date, balance: 1000, cash_balance: 1000, currency: "USD"),
|
||||
Balance.new(date: Date.current, balance: 1000, cash_balance: 1000, currency: "USD")
|
||||
]
|
||||
)
|
||||
|
||||
assert_difference "@account.balances.count", 2 do
|
||||
Balance::Syncer.new(@account, strategy: :forward).sync_balances
|
||||
end
|
||||
end
|
||||
|
||||
# BAD!!
|
||||
# Unnecessary test - in this case, this is simply testing ActiveRecord's functionality
|
||||
test "saves balance" do
|
||||
balance_record = Balance.new(balance: 100, currency: "USD")
|
||||
|
||||
assert balance_record.save
|
||||
end
|
||||
```
|
||||
|
||||
### Convention 6: Use ActiveRecord for complex validations, DB for simple ones, keep business logic out of DB
|
||||
### Convention 5: Use ActiveRecord for complex validations, DB for simple ones, keep business logic out of DB
|
||||
|
||||
- Enforce `null` checks, unique indexes, and other simple validations in the DB
|
||||
- ActiveRecord validations _may_ mirror the DB level ones, but not 100% necessary. These are for convenience when error handling in forms. Always prefer client-side form validation when possible.
|
||||
|
|
|
|||
64
.cursor/rules/stimulus_conventions.mdc
Normal file
64
.cursor/rules/stimulus_conventions.mdc
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
---
|
||||
description:
|
||||
globs:
|
||||
alwaysApply: false
|
||||
---
|
||||
This rule describes how to write Stimulus controllers.
|
||||
|
||||
- **Use declarative actions, not imperative event listeners**
|
||||
- Instead of assigning a Stimulus target and binding it to an event listener in the initializer, always write Controllers + ERB views declaratively by using Stimulus actions in ERB to call methods in the Stimulus JS controller. Below are good vs. bad code.
|
||||
|
||||
BAD code:
|
||||
|
||||
```js
|
||||
// BAD!!!! DO NOT DO THIS!!
|
||||
// Imperative - controller does all the work
|
||||
export default class extends Controller {
|
||||
static targets = ["button", "content"]
|
||||
|
||||
connect() {
|
||||
this.buttonTarget.addEventListener("click", this.toggle.bind(this))
|
||||
}
|
||||
|
||||
toggle() {
|
||||
this.contentTarget.classList.toggle("hidden")
|
||||
this.buttonTarget.textContent = this.contentTarget.classList.contains("hidden") ? "Show" : "Hide"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
GOOD code:
|
||||
|
||||
```erb
|
||||
<!-- Declarative - HTML declares what happens -->
|
||||
|
||||
<div data-controller="toggle">
|
||||
<button data-action="click->toggle#toggle" data-toggle-target="button">Show</button>
|
||||
<div data-toggle-target="content" class="hidden">Hello World!</div>
|
||||
</div>
|
||||
```
|
||||
|
||||
```js
|
||||
// Declarative - controller just responds
|
||||
export default class extends Controller {
|
||||
static targets = ["button", "content"]
|
||||
|
||||
toggle() {
|
||||
this.contentTarget.classList.toggle("hidden")
|
||||
this.buttonTarget.textContent = this.contentTarget.classList.contains("hidden") ? "Show" : "Hide"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- **Keep Stimulus controllers lightweight and simple**
|
||||
- Always aim for less than 7 controller targets. Any more is a sign of too much complexity.
|
||||
- Use private methods and expose a clear public API
|
||||
|
||||
- **Keep Stimulus controllers focused on what they do best**
|
||||
- Domain logic does NOT belong in a Stimulus controller
|
||||
- Stimulus controllers should aim for a single responsibility, or a group of highly related responsibilities
|
||||
- Make good use of Stimulus's callbacks, actions, targets, values, and classes
|
||||
|
||||
- **Component controllers should not be used outside the component**
|
||||
- If a Stimulus controller is in the app/components directory, it should only be used in its component view. It should not be used anywhere in app/views.
|
||||
|
||||
87
.cursor/rules/testing.mdc
Normal file
87
.cursor/rules/testing.mdc
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
---
|
||||
description:
|
||||
globs: test/**
|
||||
alwaysApply: false
|
||||
---
|
||||
Use this rule to learn how to write tests for the Maybe codebase.
|
||||
|
||||
Due to the open-source nature of this project, we have chosen Minitest + Fixtures for testing to maximize familiarity and predictability.
|
||||
|
||||
- **General testing rules**
|
||||
- Always use Minitest and fixtures for testing, NEVER rspec or factories
|
||||
- Keep fixtures to a minimum. Most models should have 2-3 fixtures maximum that represent the "base cases" for that model. "Edge cases" should be created on the fly, within the context of the test which it is needed.
|
||||
- For tests that require a large number of fixture records to be created, use Rails helpers to help create the records needed for the test, then inline the creation. For example, [entries_test_helper.rb](mdc:test/support/entries_test_helper.rb) provides helpers to easily do this.
|
||||
|
||||
- **Write minimal, effective tests**
|
||||
- Use system tests sparingly as they increase the time to complete the test suite
|
||||
- Only write tests for critical and important code paths
|
||||
- Write tests as you go, when required
|
||||
- Take a practical approach to testing. Tests are effective when their presence _significantly increases confidence in the codebase_.
|
||||
|
||||
Below are examples of necessary vs. unnecessary tests:
|
||||
|
||||
```rb
|
||||
# GOOD!!
|
||||
# Necessary test - in this case, we're testing critical domain business logic
|
||||
test "syncs balances" do
|
||||
Holding::Syncer.any_instance.expects(:sync_holdings).returns([]).once
|
||||
|
||||
@account.expects(:start_date).returns(2.days.ago.to_date)
|
||||
|
||||
Balance::ForwardCalculator.any_instance.expects(:calculate).returns(
|
||||
[
|
||||
Balance.new(date: 1.day.ago.to_date, balance: 1000, cash_balance: 1000, currency: "USD"),
|
||||
Balance.new(date: Date.current, balance: 1000, cash_balance: 1000, currency: "USD")
|
||||
]
|
||||
)
|
||||
|
||||
assert_difference "@account.balances.count", 2 do
|
||||
Balance::Syncer.new(@account, strategy: :forward).sync_balances
|
||||
end
|
||||
end
|
||||
|
||||
# BAD!!
|
||||
# Unnecessary test - in this case, this is simply testing ActiveRecord's functionality
|
||||
test "saves balance" do
|
||||
balance_record = Balance.new(balance: 100, currency: "USD")
|
||||
|
||||
assert balance_record.save
|
||||
end
|
||||
```
|
||||
|
||||
- **Test boundaries correctly**
|
||||
- Distinguish between commands and query methods. Test output of query methods; test that commands were called with the correct params. See an example below:
|
||||
|
||||
```rb
|
||||
class ExampleClass
|
||||
def do_something
|
||||
result = 2 + 2
|
||||
|
||||
CustomEventProcessor.process_result(result)
|
||||
|
||||
result
|
||||
end
|
||||
end
|
||||
|
||||
class ExampleClass < ActiveSupport::TestCase
|
||||
test "boundaries are tested correctly" do
|
||||
result = ExampleClass.new.do_something
|
||||
|
||||
# GOOD - we're only testing that the command was received, not internal implementation details
|
||||
# The actual tests for CustomEventProcessor belong in a different test suite!
|
||||
CustomEventProcessor.expects(:process_result).with(4).once
|
||||
|
||||
# GOOD - we're testing the implementation of ExampleClass inside its own test suite
|
||||
assert_equal 4, result
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
- Never test the implementation details of one class in another classes test suite
|
||||
|
||||
- **Stubs and mocks**
|
||||
- Use `mocha` gem
|
||||
- Always prefer `OpenStruct` when creating mock instances, or in complex cases, a mock class
|
||||
- Only mock what's necessary. If you're not testing return values, don't mock a return value.
|
||||
|
||||
|
||||
100
.cursor/rules/view_conventions.mdc
Normal file
100
.cursor/rules/view_conventions.mdc
Normal file
|
|
@ -0,0 +1,100 @@
|
|||
---
|
||||
description:
|
||||
globs: app/views/**,app/javascript/**,app/components/**/*.js
|
||||
alwaysApply: false
|
||||
---
|
||||
Use this rule to learn how to write ERB views, partials, and Stimulus controllers should be incorporated into them.
|
||||
|
||||
- **Component vs. Partial Decision Making**
|
||||
- **Use ViewComponents when:**
|
||||
- Element has complex logic or styling patterns
|
||||
- Element will be reused across multiple views/contexts
|
||||
- Element needs structured styling with variants/sizes (like buttons, badges)
|
||||
- Element requires interactive behavior or Stimulus controllers
|
||||
- Element has configurable slots or complex APIs
|
||||
- Element needs accessibility features or ARIA support
|
||||
|
||||
- **Use Partials when:**
|
||||
- Element is primarily static HTML with minimal logic
|
||||
- Element is used in only one or few specific contexts
|
||||
- Element is simple template content (like CTAs, static sections)
|
||||
- Element doesn't need variants, sizes, or complex configuration
|
||||
- Element is more about content organization than reusable functionality
|
||||
|
||||
- **Prefer components over partials**
|
||||
- If there is a component available for the use case in app/components, use it
|
||||
- If there is no component, look for a partial
|
||||
- If there is no partial, decide between component or partial based on the criteria above
|
||||
|
||||
- **Examples of Component vs. Partial Usage**
|
||||
```erb
|
||||
<%# Component: Complex, reusable with variants and interactivity %>
|
||||
<%= render DialogComponent.new(variant: :drawer) do |dialog| %>
|
||||
<% dialog.with_header(title: "Account Settings") %>
|
||||
<% dialog.with_body { "Dialog content here" } %>
|
||||
<% end %>
|
||||
|
||||
<%# Component: Interactive with complex styling options %>
|
||||
<%= render ButtonComponent.new(text: "Save Changes", variant: "primary", confirm: "Are you sure?") %>
|
||||
|
||||
<%# Component: Reusable with variants %>
|
||||
<%= render FilledIconComponent.new(icon: "credit-card", variant: :surface) %>
|
||||
|
||||
<%# Partial: Static template content %>
|
||||
<%= render "shared/logo" %>
|
||||
|
||||
<%# Partial: Simple, context-specific content with basic styling %>
|
||||
<%= render "shared/trend_change", trend: @account.trend, comparison_label: "vs last month" %>
|
||||
|
||||
<%# Partial: Simple divider/utility %>
|
||||
<%= render "shared/ruler", classes: "my-4" %>
|
||||
|
||||
<%# Partial: Simple form utility %>
|
||||
<%= render "shared/form_errors", model: @account %>
|
||||
```
|
||||
|
||||
- **Keep domain logic out of the views**
|
||||
```erb
|
||||
<%# BAD!!! %>
|
||||
|
||||
<%# This belongs in the component file, not the template file! %>
|
||||
<% button_classes = { class: "bg-blue-500 hover:bg-blue-600" } %>
|
||||
|
||||
<%= tag.button class: button_classes do %>
|
||||
Save Account
|
||||
<% end %>
|
||||
|
||||
<%# GOOD! %>
|
||||
|
||||
<%= tag.button class: computed_button_classes do %>
|
||||
Save Account
|
||||
<% end %>
|
||||
```
|
||||
|
||||
- **Stimulus Integration in Views**
|
||||
- Always use the **declarative approach** when integrating Stimulus controllers
|
||||
- The ERB template should declare what happens, the Stimulus controller should respond
|
||||
- Refer to [stimulus_conventions.mdc](mdc:.cursor/rules/stimulus_conventions.mdc) to learn how to incorporate them into
|
||||
|
||||
GOOD Stimulus controller integration into views:
|
||||
|
||||
```erb
|
||||
<!-- Declarative - HTML declares what happens -->
|
||||
|
||||
<div data-controller="toggle">
|
||||
<button data-action="click->toggle#toggle" data-toggle-target="button">Show</button>
|
||||
<div data-toggle-target="content" class="hidden">Hello World!</div>
|
||||
</div>
|
||||
```
|
||||
|
||||
- **Stimulus Controller Placement Guidelines**
|
||||
- **Component controllers** (in `app/components/`) should only be used within their component templates
|
||||
- **Global controllers** (in `app/javascript/controllers/`) can be used across any view
|
||||
- Pass data from Rails to Stimulus using `data-*-value` attributes, not inline JavaScript
|
||||
- Use Stimulus targets to reference DOM elements, not manual `getElementById` calls
|
||||
|
||||
- **Naming Conventions**
|
||||
- **Components**: Use `ComponentName` suffix (e.g., `ButtonComponent`, `DialogComponent`, `FilledIconComponent`)
|
||||
- **Partials**: Use underscore prefix (e.g., `_trend_change.html.erb`, `_form_errors.html.erb`, `_sync_indicator.html.erb`)
|
||||
- **Shared partials**: Place in `app/views/shared/` directory for reusable content
|
||||
- **Context-specific partials**: Place in relevant controller view directory (e.g., `accounts/_account_sidebar_tabs.html.erb`)
|
||||
Loading…
Add table
Add a link
Reference in a new issue