From 908b3e24899a69591de8db641522de9eac51b209 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 12 May 2025 15:41:14 -0400 Subject: [PATCH 1/6] Temporary disable of sync cascade behavior --- app/models/sync.rb | 49 ++++------------------------------------ test/models/sync_test.rb | 40 -------------------------------- 2 files changed, 4 insertions(+), 85 deletions(-) diff --git a/app/models/sync.rb b/app/models/sync.rb index 826899ce..4d923f12 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -21,58 +21,17 @@ class Sync < ApplicationRecord begin syncable.sync_data(self, start_date: start_date) - unless has_pending_child_syncs? - complete! - Rails.logger.info("Sync completed, starting post-sync") - syncable.post_sync(self) - Rails.logger.info("Post-sync completed") - end + complete! + Rails.logger.info("Sync completed, starting post-sync") + syncable.post_sync(self) + Rails.logger.info("Post-sync completed") rescue StandardError => error fail! error, report_error: true - ensure - notify_parent_of_completion! if has_parent? - end - end - end - - def handle_child_completion_event - Sync.transaction do - # We need this to ensure 2 child syncs don't update the parent at the exact same time with different results - # and cause the sync to hang in "syncing" status indefinitely - self.lock! - - unless has_pending_child_syncs? - if has_failed_child_syncs? - fail!(Error.new("One or more child syncs failed")) - else - complete! - end - - # If this sync is both a child and a parent, we need to notify the parent of completion - notify_parent_of_completion! if has_parent? - - syncable.post_sync(self) end end end private - def has_pending_child_syncs? - children.where(status: [ :pending, :syncing ]).any? - end - - def has_failed_child_syncs? - children.where(status: :failed).any? - end - - def has_parent? - parent_id.present? - end - - def notify_parent_of_completion! - parent.handle_child_completion_event - end - def start! Rails.logger.info("Starting sync") update! status: :syncing diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 6b246a1f..99019146 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -31,44 +31,4 @@ class SyncTest < ActiveSupport::TestCase assert_equal "failed", @sync.status assert_equal "test sync error", @sync.error end - - # Order is important here. Parent syncs must implement sync_data so that their own work - # is 100% complete *prior* to queueing up child syncs. - test "runs sync with child syncs" do - family = families(:dylan_family) - - parent = Sync.create!(syncable: family) - child1 = Sync.create!(syncable: family.accounts.first, parent: parent) - child2 = Sync.create!(syncable: family.accounts.second, parent: parent) - grandchild = Sync.create!(syncable: family.accounts.last, parent: child2) - - parent.syncable.expects(:sync_data).returns([]).once - child1.syncable.expects(:sync_data).returns([]).once - child2.syncable.expects(:sync_data).returns([]).once - grandchild.syncable.expects(:sync_data).returns([]).once - - assert_equal "pending", parent.status - assert_equal "pending", child1.status - assert_equal "pending", child2.status - assert_equal "pending", grandchild.status - - parent.perform - assert_equal "syncing", parent.reload.status - - child1.perform - assert_equal "completed", child1.reload.status - assert_equal "syncing", parent.reload.status - - child2.perform - assert_equal "syncing", child2.reload.status - assert_equal "completed", child1.reload.status - assert_equal "syncing", parent.reload.status - - # Will complete the parent and grandparent syncs - grandchild.perform - assert_equal "completed", grandchild.reload.status - assert_equal "completed", child1.reload.status - assert_equal "completed", child2.reload.status - assert_equal "completed", parent.reload.status - end end From 9e6e4b1ce6937879d0061ca5ab845bf8b629652f Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 12 May 2025 18:55:19 -0400 Subject: [PATCH 2/6] Only run Plaid syncs via webhook after initial sync --- app/models/family.rb | 5 ----- app/views/accounts/index.html.erb | 19 ++++++++++--------- app/views/accounts/show/_header.html.erb | 17 +++-------------- app/views/plaid_items/_plaid_item.html.erb | 2 +- test/models/family_test.rb | 4 ---- 5 files changed, 14 insertions(+), 33 deletions(-) diff --git a/app/models/family.rb b/app/models/family.rb index f565d54d..a3a73eec 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -74,11 +74,6 @@ class Family < ApplicationRecord account.sync_later(start_date: start_date, parent_sync: sync) end - Rails.logger.info("Syncing plaid items for family #{id}") - plaid_items.each do |plaid_item| - plaid_item.sync_later(start_date: start_date, parent_sync: sync) - end - Rails.logger.info("Applying rules for family #{id}") rules.each do |rule| rule.apply_later diff --git a/app/views/accounts/index.html.erb b/app/views/accounts/index.html.erb index 4893e44c..b4c78332 100644 --- a/app/views/accounts/index.html.erb +++ b/app/views/accounts/index.html.erb @@ -2,15 +2,16 @@

<%= t(".accounts") %>

- <%= render ButtonComponent.new( - text: "Sync all", - href: sync_all_accounts_path, - method: :post, - variant: "outline", - disabled: Current.family.syncing?, - icon: "refresh-cw", - class: "" - ) %> + <% if Rails.env.development? %> + <%= render ButtonComponent.new( + text: "Sync all", + href: sync_all_accounts_path, + method: :post, + variant: "outline", + disabled: Current.family.syncing?, + icon: "refresh-cw", + ) %> + <% end %> <%= render LinkComponent.new( text: "New account", diff --git a/app/views/accounts/show/_header.html.erb b/app/views/accounts/show/_header.html.erb index 4c60e832..b64408ce 100644 --- a/app/views/accounts/show/_header.html.erb +++ b/app/views/accounts/show/_header.html.erb @@ -20,26 +20,15 @@ <% end %>
- <% if account.plaid_account_id.present? %> - <% if Rails.env.development? %> - <%= icon( + <% if Rails.env.development? %> + <%= icon( "refresh-cw", as_button: true, size: "sm", - href: sync_plaid_item_path(account.plaid_account.plaid_item), + href: account.linked? ? sync_plaid_item_path(account.plaid_account.plaid_item) : sync_account_path(account), disabled: account.syncing?, frame: :_top ) %> - <% end %> - <% else %> - <%= icon( - "refresh-cw", - as_button: true, - size: "sm", - href: sync_account_path(account), - disabled: account.syncing?, - frame: :_top - ) %> <% end %> <%= render "accounts/show/menu", account: account %> diff --git a/app/views/plaid_items/_plaid_item.html.erb b/app/views/plaid_items/_plaid_item.html.erb index 7c3dc8b2..61dea7dc 100644 --- a/app/views/plaid_items/_plaid_item.html.erb +++ b/app/views/plaid_items/_plaid_item.html.erb @@ -92,7 +92,7 @@
<% end %> - <% else %> + <% elsif Rails.env.development? %> <%= icon( "refresh-cw", as_button: true, diff --git a/test/models/family_test.rb b/test/models/family_test.rb index 7223e64b..24876a77 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -20,10 +20,6 @@ class FamilyTest < ActiveSupport::TestCase .with(start_date: nil, parent_sync: family_sync) .times(manual_accounts_count) - PlaidItem.any_instance.expects(:sync_later) - .with(start_date: nil, parent_sync: family_sync) - .times(items_count) - @syncable.sync_data(family_sync, start_date: family_sync.start_date) end end From 0fb689290a2199272688b4f832cfbbef5e77eba2 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 13 May 2025 08:32:53 -0400 Subject: [PATCH 3/6] Family subscription unique index --- .../20250513122703_add_uniqueness_to_subscriptions.rb | 6 ++++++ db/schema.rb | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20250513122703_add_uniqueness_to_subscriptions.rb diff --git a/db/migrate/20250513122703_add_uniqueness_to_subscriptions.rb b/db/migrate/20250513122703_add_uniqueness_to_subscriptions.rb new file mode 100644 index 00000000..f97b67f9 --- /dev/null +++ b/db/migrate/20250513122703_add_uniqueness_to_subscriptions.rb @@ -0,0 +1,6 @@ +class AddUniquenessToSubscriptions < ActiveRecord::Migration[7.2] + def change + remove_index :subscriptions, :family_id + add_index :subscriptions, :family_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 6d76a125..c9ba49e5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_05_09_182903) do +ActiveRecord::Schema[7.2].define(version: 2025_05_13_122703) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -580,7 +580,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_09_182903) do t.datetime "trial_ends_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["family_id"], name: "index_subscriptions_on_family_id" + t.index ["family_id"], name: "index_subscriptions_on_family_id", unique: true end create_table "syncs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| From df8e22afe9e56341365e7d7e0b2b58141949ad33 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 13 May 2025 08:56:32 -0400 Subject: [PATCH 4/6] Stripe tasks --- lib/tasks/stripe.rake | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 lib/tasks/stripe.rake diff --git a/lib/tasks/stripe.rake b/lib/tasks/stripe.rake new file mode 100644 index 00000000..7e2124de --- /dev/null +++ b/lib/tasks/stripe.rake @@ -0,0 +1,28 @@ +namespace :stripe do + desc "Sync legacy Stripe subscriptions" + task sync_legacy_subscriptions: :environment do + cli = Stripe::StripeClient.new(ENV["STRIPE_SECRET_KEY"]) + + subs = cli.v1.subscriptions.list + + subs.auto_paging_each do |sub| + details = sub.items.data.first + + family = Family.find_by(stripe_customer_id: sub.customer) + + if family.nil? + puts "Family not found for Stripe customer ID: #{sub.customer}, skipping" + next + end + + family.subscription.update!( + stripe_id: sub.id, + status: sub.status, + interval: details.plan.interval, + amount: details.plan.amount / 100.0, + currency: details.plan.currency.upcase, + current_period_ends_at: Time.at(details.current_period_end) + ) + end + end +end From 30d3eef67fc7e8b1d421a788eaa04bcce0fec59e Mon Sep 17 00:00:00 2001 From: Alex Hatzenbuhler Date: Tue, 13 May 2025 09:34:41 -0500 Subject: [PATCH 5/6] Fix AND prefix on rule form (#2234) * Fix AND prefix on rule form This new condition prefix data target is used to ensure the AND prefix is added/removed to additional conditions/groups when there aren't any saved conditions yet. * Ensure the condition group "Add condition" button is type button to avoid form submission * Add prefix update when removing a subcondition * Use data condition to update the prefix on conditions, condition groups, and subconditions * Use condition remove instead of element remove for condition groups so prefix logic runs * Add back explicit show_prefixes to ensure subconditions never have a prefix * Run the prefix update runs on a load of a form, which handles prefixes on an edit since no conditions change * Ensure saved items that are marked for removal don't impact the index * Simplify logic since we don't process subconditions * Clean up comments * Add primary_condition_title field to rule model --- .../controllers/rule/conditions_controller.js | 11 +++++++ .../controllers/rules_controller.js | 30 +++++++++++++++++++ app/models/rule.rb | 12 ++++++++ app/views/rule/conditions/_condition.html.erb | 7 +++-- .../rule/conditions/_condition_group.html.erb | 18 +++++------ app/views/rules/_form.html.erb | 2 +- app/views/rules/_rule.html.erb | 8 ++--- 7 files changed, 70 insertions(+), 18 deletions(-) diff --git a/app/javascript/controllers/rule/conditions_controller.js b/app/javascript/controllers/rule/conditions_controller.js index 1cffa119..d0c12941 100644 --- a/app/javascript/controllers/rule/conditions_controller.js +++ b/app/javascript/controllers/rule/conditions_controller.js @@ -21,12 +21,23 @@ export default class extends Controller { } remove(e) { + // Find the parent rules controller before removing the condition + const rulesEl = this.element.closest('[data-controller~="rules"]'); + if (e.params.destroy) { this.destroyFieldTarget.value = true; this.element.classList.add("hidden"); } else { this.element.remove(); } + + // Update the prefixes of all conditions from the parent rules controller + if (rulesEl) { + const rulesController = this.application.getControllerForElementAndIdentifier(rulesEl, "rules"); + if (rulesController && typeof rulesController.updateConditionPrefixes === "function") { + rulesController.updateConditionPrefixes(); + } + } } handleConditionTypeChange(e) { diff --git a/app/javascript/controllers/rules_controller.js b/app/javascript/controllers/rules_controller.js index 0db0e67a..3618acc6 100644 --- a/app/javascript/controllers/rules_controller.js +++ b/app/javascript/controllers/rules_controller.js @@ -11,11 +11,17 @@ export default class extends Controller { "effectiveDateInput", ]; + connect() { + // Update condition prefixes on first connection (form render on edit) + this.updateConditionPrefixes(); + } + addConditionGroup() { this.#appendTemplate( this.conditionGroupTemplateTarget, this.conditionsListTarget, ); + this.updateConditionPrefixes(); } addCondition() { @@ -23,6 +29,7 @@ export default class extends Controller { this.conditionTemplateTarget, this.conditionsListTarget, ); + this.updateConditionPrefixes(); } addAction() { @@ -45,4 +52,27 @@ export default class extends Controller { #uniqueKey() { return Date.now(); } + + // Updates the prefix visibility of all conditions and condition groups + // This is also called by the rule/conditions_controller when a subcondition is removed + updateConditionPrefixes() { + const conditions = Array.from(this.conditionsListTarget.children); + let conditionIndex = 0; + + conditions.forEach((condition) => { + // Only process visible conditions, this prevents conditions that are marked for removal and hidden + // from being added to the index. This is important when editing a rule. + if (!condition.classList.contains('hidden')) { + const prefixEl = condition.querySelector('[data-condition-prefix]'); + if (prefixEl) { + if (conditionIndex === 0) { + prefixEl.classList.add('hidden'); + } else { + prefixEl.classList.remove('hidden'); + } + conditionIndex++; + } + } + }); + } } diff --git a/app/models/rule.rb b/app/models/rule.rb index db8a99ae..ec15d64b 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -46,6 +46,18 @@ class Rule < ApplicationRecord RuleJob.perform_later(self, ignore_attribute_locks: ignore_attribute_locks) end + def primary_condition_title + return "No conditions" if conditions.none? + + first_condition = conditions.first + if first_condition.compound? && first_condition.sub_conditions.any? + first_sub_condition = first_condition.sub_conditions.first + "If #{first_sub_condition.filter.label.downcase} #{first_sub_condition.operator} #{first_sub_condition.value_display}" + else + "If #{first_condition.filter.label.downcase} #{first_condition.operator} #{first_condition.value_display}" + end + end + private def matching_resources_scope scope = registry.resource_scope diff --git a/app/views/rule/conditions/_condition.html.erb b/app/views/rule/conditions/_condition.html.erb index b79978a1..60c38eab 100644 --- a/app/views/rule/conditions/_condition.html.erb +++ b/app/views/rule/conditions/_condition.html.erb @@ -4,8 +4,11 @@ <% rule = condition.rule %>
  • - <% if form.index.to_i > 0 && show_prefix %> -
    + + <%# Conditionally render the prefix %> + <%# Condition groups pass in show_prefix: false for subconditions since the ANY/ALL selector makes that clear %> + <% if show_prefix %> +
    and
    <% end %> diff --git a/app/views/rule/conditions/_condition_group.html.erb b/app/views/rule/conditions/_condition_group.html.erb index 67b3eb0f..e04a09f7 100644 --- a/app/views/rule/conditions/_condition_group.html.erb +++ b/app/views/rule/conditions/_condition_group.html.erb @@ -3,17 +3,16 @@ <% condition = form.object %> <% rule = condition.rule %> -
  • +
  • <%= form.hidden_field :condition_type, value: "compound" %>
    - <% unless form.index == 0 %> -
    - and -
    - <% end %> + <%# Show prefix on condition groups, except the first one %> +
    + and +

    match

    <%= form.select :operator, [["all", "and"], ["any", "or"]], { container_class: "w-fit" }, data: { rules_target: "operatorField" } %>

    of the following conditions

    @@ -21,16 +20,16 @@ <%= icon( "trash-2", - size: "sm", as_button: true, - data: { action: "element-removal#remove" } + size: "sm", + data: { action: "rule--conditions#remove" } ) %>
    <%# Sub-condition template, used by Stimulus controller to add new sub-conditions dynamically %> @@ -44,6 +43,7 @@ text: "Add condition", leading_icon: "plus", variant: "ghost", + type: "button", data: { action: "rule--conditions#addSubCondition" } ) %>
    • diff --git a/app/views/rules/_form.html.erb b/app/views/rules/_form.html.erb index 79c0b45e..cdee4783 100644 --- a/app/views/rules/_form.html.erb +++ b/app/views/rules/_form.html.erb @@ -12,7 +12,7 @@

    If <%= rule.resource_type %>

    - <%# Condition template, used by Stimulus controller to add new conditions dynamically %> + <%# Condition Group template, used by Stimulus controller to add new conditions dynamically %>