Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: NoMethodError (MAYBE-RAILS-D1) #1688

Closed
wants to merge 1 commit into from

Conversation

revise-dev[bot]
Copy link

@revise-dev revise-dev bot commented Jan 25, 2025

This error occurs because the weight method in Account::Holding attempts to calculate a percentage based on the account's balance, but the account association is nil in the test data. The original intent of the test was to verify that balance calculation works correctly with investment holdings, but it failed to properly set up the test data with the required account association.

Key changes made:

  1. Added account: @account to each Account::Holding.new call in the test data setup to ensure the account association is properly set
  2. Maintained all other attributes (date, security, amount) as they were originally defined
  3. Kept the original test assertions intact since they were correct, just fixing the setup data

The test is verifying that:

  • Balance calculation works correctly when an account has investment holdings
  • Both forward and backward calculations produce the same results
  • The balance transitions correctly through different dates based on trades and holdings

The fix ensures that when the weight calculation is performed in Account::Holding#weight (which divides amount by account.balance), it has access to a valid account object and its balance. This maintains the original intent of calculating the holding's percentage of the total account value while preventing the nil reference error.

The test itself is part of a comprehensive suite testing balance calculations under various scenarios (transactions, valuations, multi-currency, etc.), and this particular test focuses on investment-specific balance calculations where both cash and security holdings need to be considered.

Tip

You can make revisions or ask questions of Revise.dev by using /revise in any comment or review!

  • /revise Add a comment above the method to explain why we're making this change.
  • /revise Why did you choose to make this change specifically?

Important

If something doesn't look right, click to retry this interaction.

@zachgoll zachgoll closed this Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant