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

Synth error handling #1502

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Synth error handling #1502

merged 3 commits into from
Nov 26, 2024

Conversation

Shpigford
Copy link
Member

No description provided.

@Shpigford Shpigford requested a review from zachgoll November 25, 2024 20:53
@Shpigford
Copy link
Member Author

@zachgoll Not sure if I'm using the error stuff correctly. Mind taking a look?

@Shpigford Shpigford marked this pull request as ready for review November 25, 2024 21:00
@zachgoll
Copy link
Collaborator

zachgoll commented Nov 25, 2024

@Shpigford to get this working, there are a few additional steps:

First, we'll need to implement the stale? method on the issue:

class Issue::SynthLimitExceeded < Issue
  include Providable # Give this class access to our configured Synth Provider

  def title
    "Synth API Credit Limit Exceeded"
  end
 
  # Required so our app knows if the error is still happening after the user has taken some action to fix it
  def stale?
    self.class.synth_provider.usage.utilization < 1
  end 
end

Then, we need to add a method in issuable.rb to observe this error:

# app/models/concerns/issuable.rb

def observe_synth_credits_overage
  observe_issue(Issue::SynthLimitExceeded.new)
end

And finally, we need to call account.observe_synth_credits_overage during the account sync process:

class Account
   include Providable 

   def sync_data(start_date: nil)
    update!(last_synced_at: Time.current)

    resolve_stale_issues

    # Observe issue if credits are over limit
    if self.class.synth_provider.usage.utilization >= 1
      observe_synth_credits_overage
    end

    Balance::Syncer.new(self, start_date: start_date).run
    Holding::Syncer.new(self, start_date: start_date).run
  end
end

Simpler solution

I think given this use case, there are probably some simplifications that could be made to the "issues" flow and the Provider::Synth to make all of that more seamless (I can circle back to this after I get investment syncs squared away).

In the meantime, a simpler solution would be to add a convenience method on Family and then conditionally show some alert message somewhere in the UI when they are over the limit:

class Family 
  include Providable 

  def synth_overage?
    self.class.synth_provider && self.class.synth_provider.usage.utilization >= 1
  end
end
<% if Current.family.synth_overage? %> 
  <div>Some alert to user!!!</div>
<% end %> 

@Shpigford
Copy link
Member Author

@zachgoll I definitely prefer the simple approach. 🙂 Will get that swapped out and have you review in a bit.

@zachgoll
Copy link
Collaborator

@Shpigford yeah I agree. Longer term, I think we will basically have 1 single Issue::SynthIssue < Issue that will cover all the possible Synth errors and provide UIs for dealing with them, but for now it definitely needs some fine-tuning to avoid adding a bunch of unnecessary complexity

@Shpigford
Copy link
Member Author

@zachgoll Mind taking a look at this version?

Copy link
Collaborator

@zachgoll zachgoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks good

@Shpigford Shpigford merged commit a9b61a6 into main Nov 26, 2024
5 checks passed
@Shpigford Shpigford deleted the synth-overage branch November 26, 2024 13:45
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.

2 participants