-
Notifications
You must be signed in to change notification settings - Fork 893
Add comprehensive bond accounts and API integration #1330
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
base: main
Are you sure you want to change the base?
Changes from 72 commits
707d163
95c2027
9d23209
f065d6a
b5c70fd
22f6c3e
cae4c33
a9818ed
7cd865b
166f7a4
d425419
fb36c81
d0eb273
50997f6
7ccd1fb
b27d0d9
9c71a46
9e0b341
a76162b
01ee0e9
774ec7a
e053217
02a13fa
e0e5111
0117b69
2927030
0285c09
47696cb
535bca4
c452737
d6dbb04
5b33646
d4b0072
575daf0
07f4c6a
9502096
a29e329
dedc9fa
29e330e
f97713f
ec1e9a7
dbd634e
13ed045
2791f95
596c8c3
3f2851f
e9a2a5d
d5efd3b
fc00f6a
8ab59c9
29d7cec
0cc44ac
dca603a
1f822e6
d948dd3
b6ed51a
1d92928
7615e81
7df6101
37ce1c0
f280e33
ecd0922
6e1ca73
22292a8
7502dd9
9fcbb3e
f0bcd0c
32e306d
4ef188f
d21122e
435e013
5dd3b78
20fcc1c
3c139d5
ed5843f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <tr class="<%= row_classes %>"> | ||
| <td class="px-4 py-4"> | ||
| <div class="flex items-center gap-4"> | ||
| <%= render DS::FilledIcon.new(variant: :text, text: subtype_label, size: "md", rounded: true) %> | ||
| <div class="space-y-0.5 min-w-0"> | ||
| <%= link_to subtype_label, account_path(account, tab: "positions"), class: "truncate hover:underline" %> | ||
| <p class="text-xs text-secondary truncate"><%= t("pages.dashboard.bond_summary.account_wrapper", account: account.name, wrapper: account.bond_wrapper_label || account.short_subtype_label) %></p> | ||
| </div> | ||
| </div> | ||
| </td> | ||
|
|
||
| <td class="px-4 py-4 text-right"> | ||
| <p class="privacy-sensitive"><%= rate_text %></p> | ||
| <p class="font-normal text-secondary"><%= rate_meta %></p> | ||
| </td> | ||
|
|
||
| <td class="px-4 py-4 text-right"> | ||
| <p class="privacy-sensitive"><%= helpers.format_money(Money.new(lot.amount, account.currency)) %></p> | ||
| <p class="font-normal text-secondary"><%= t("pages.dashboard.bond_summary.principal_term", term: t("pages.dashboard.bond_summary.term_months", count: lot.term_months)) %></p> | ||
| </td> | ||
|
|
||
| <td class="px-4 py-4 text-right"> | ||
| <p class="privacy-sensitive"><%= lot.maturity_date ? l(lot.maturity_date) : t("bonds.purchase_holding.unknown") %></p> | ||
| <p class="font-normal text-secondary"><%= t("pages.dashboard.bond_summary.maturity_label") %></p> | ||
| </td> | ||
|
|
||
| <td class="px-4 py-4 text-right"> | ||
| <p class="privacy-sensitive <%= total_return_class %>"><%= helpers.format_money(Money.new(total_return_amount, account.currency)) %></p> | ||
| <p class="font-normal text-secondary"><%= total_return_label %></p> | ||
| </td> | ||
| </tr> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,143 @@ | ||||||||||||||||||||||||||||||||||||
| class UI::Dashboard::BondSummaryRow < ApplicationComponent | ||||||||||||||||||||||||||||||||||||
| attr_reader :account, :lot, :show_border | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def initialize(account:, lot:, show_border: false) | ||||||||||||||||||||||||||||||||||||
| @account = account | ||||||||||||||||||||||||||||||||||||
| @lot = lot | ||||||||||||||||||||||||||||||||||||
| @show_border = show_border | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def subtype_label | ||||||||||||||||||||||||||||||||||||
| Bond.long_subtype_label_for(lot.subtype) || t("bonds.purchase_holding.unknown") | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def total_return_amount | ||||||||||||||||||||||||||||||||||||
| @total_return_amount ||= projected_total_return? ? projected_return_amount : current_return_amount | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def total_return_label | ||||||||||||||||||||||||||||||||||||
| if projected_total_return? | ||||||||||||||||||||||||||||||||||||
| t("bonds.purchase_holding.projected_to_maturity") | ||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||
| t("bonds.purchase_holding.since_purchase") | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def total_return_class | ||||||||||||||||||||||||||||||||||||
| total_return_amount.negative? ? "text-destructive" : "text-success" | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential Based on the model code, both Suggested fix def total_return_class
- total_return_amount.negative? ? "text-destructive" : "text-success"
+ return "text-secondary" if total_return_amount.nil?
+
+ total_return_amount.negative? ? "text-destructive" : "text-success"
endYou may also want to handle the nil case in the view template to avoid rendering invalid return amounts. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| def rate_text | ||||||||||||||||||||||||||||||||||||
| if lot.inflation_linked? | ||||||||||||||||||||||||||||||||||||
| return t("bonds.purchase_holding.update_needed") if lot.requires_rate_review? | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| current_rate = lot.current_rate_percent(allow_import: false) | ||||||||||||||||||||||||||||||||||||
| return "#{current_rate.round(3)}%" if current_rate.present? | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| t("bonds.purchase_holding.unknown") | ||||||||||||||||||||||||||||||||||||
|
UberDudePL marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||
| rate = lot.interest_rate | ||||||||||||||||||||||||||||||||||||
| rate.present? ? "#{(rate * 100).round(3)}%" : t("bonds.purchase_holding.unknown") | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| rate.present? ? "#{(rate * 100).round(3)}%" : t("bonds.purchase_holding.unknown") | |
| rate.present? ? "#{rate.round(3)}%" : t("bonds.purchase_holding.unknown") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nil values will cause NoMethodError in predicate logic.
Both current_return_amount and projected_return_amount can be nil (per context snippets showing the underlying model methods return nil when data is missing). Calling .abs or .positive? on nil raises NoMethodError.
Suggested fix with nil guards
def projected_total_return?
return `@projected_total_return` if defined?(`@projected_total_return`)
- `@projected_total_return` = current_return_amount.abs < 0.01.to_d &&
- projected_return_amount.positive?
+ current = current_return_amount
+ projected = projected_return_amount
+
+ `@projected_total_return` = current.present? &&
+ projected.present? &&
+ current.abs < 0.01.to_d &&
+ projected.positive?
endThis ensures the method returns false when data is unavailable rather than raising an exception.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def projected_total_return? | |
| return @projected_total_return if defined?(@projected_total_return) | |
| @projected_total_return = current_return_amount.abs < 0.01.to_d && | |
| projected_return_amount.positive? | |
| end | |
| def projected_total_return? | |
| return `@projected_total_return` if defined?(`@projected_total_return`) | |
| current = current_return_amount | |
| projected = projected_return_amount | |
| `@projected_total_return` = current.present? && | |
| projected.present? && | |
| current.abs < 0.01.to_d && | |
| projected.positive? | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/UI/dashboard/bond_summary_row.rb` around lines 71 - 76, The
predicate method projected_total_return? should guard against nils on
current_return_amount and projected_return_amount to avoid NoMethodError; update
projected_total_return? (and its memoization `@projected_total_return`) to return
false if either current_return_amount or projected_return_amount is nil, and
only call numeric methods (.abs, .positive?, .to_d) when those values are
present (e.g., use explicit nil checks or safe navigation) so missing data
yields false instead of raising.
Uh oh!
There was an error while loading. Please reload this page.