Skip to content

Add user credit warning for sofia_account provider#1167

Merged
lodewiges merged 3 commits intostagingfrom
feature/sofia-user-credit-warning
Dec 11, 2025
Merged

Add user credit warning for sofia_account provider#1167
lodewiges merged 3 commits intostagingfrom
feature/sofia-user-credit-warning

Conversation

@lodewiges
Copy link
Copy Markdown
Contributor

@lodewiges lodewiges commented Dec 11, 2025

This PR is needed so manual added users also need a saldo when order

Copilot AI review requested due to automatic review settings December 11, 2025 17:05
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 11, 2025

Warning

Rate limit exceeded

@lodewiges has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 39 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 45e185f and 81c46d8.

📒 Files selected for processing (2)
  • app/controllers/orders_controller.rb (1 hunks)
  • app/models/user.rb (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The pull request expands the credit insufficiency check in the user model to include sofia_account provider alongside the existing amber_oauth2 provider, affecting downstream order eligibility logic. Additionally, the user profile view now conditionally displays role configuration guidance based on whether an Amber API URL is configured.

Changes

Cohort / File(s) Summary
Credit Insufficiency Logic
app/models/user.rb
Modified insufficient_credit method to evaluate negative credit for both amber_oauth2 and sofia_account providers using explicit OR logic with grouped conditions. Previously only amber_oauth2 was checked.
Conditional Role Explanation
app/views/users/show.html.erb
Added conditional rendering under "Rollen en rechten" section to display provider-specific role configuration guidance. Shows linked-site group determination when Amber API URL is present; otherwise displays service provider console configuration guidance.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • app/models/user.rb: Verify that the expanded provider condition (sofia_account) aligns with business requirements and doesn't introduce unintended order blocking for sofia_account users with negative credit.
  • app/views/users/show.html.erb: Confirm that the conditional logic correctly detects Amber API URL configuration and that both text variants are accurate for their respective scenarios.

Poem

🐰 A rabbit hops through credit checks,
Sofia joins Amber, what comes next?
Roles now whisper, wise and bright,
Config flows where configured right!
Logic grows, the view takes care,
Changes dance through the cleaner air!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning The PR description is incomplete and lacks required information from the template. Only a one-line description is provided. Complete the PR description by filling in the Summary section (with details about UI changes and related issues using 'fixes #xyz'), Other information section, and verify the checklist items for database migrations if applicable.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: extending the insufficient_credit logic to include sofia_account provider alongside amber_oauth2.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8599e24 and 45e185f.

⛔ Files ignored due to path filters (1)
  • public/images/alphalogonotextwhite.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • app/models/user.rb (1 hunks)
  • app/views/users/show.html.erb (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Agent
  • GitHub Check: Build
🔇 Additional comments (1)
app/views/users/show.html.erb (1)

155-159: Implementation is correct, but clarify connection to PR objective.

The conditional rendering based on Amber API URL configuration is well-implemented and provides better UX. However, this change appears to address role configuration guidance rather than user credit warnings mentioned in the PR title and objectives.

Could you clarify how this view change relates to adding credit warnings for sofia_account users? If this is an additional improvement for sofia_account provider support, consider mentioning it in the PR description for better traceability.

Comment thread app/models/user.rb
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the credit warning system to support the sofia_account authentication provider, ensuring that users authenticating via sofia_account receive the same credit notifications as amber_oauth2 users.

Key Changes:

  • Extended the insufficient_credit method to include sofia_account provider alongside amber_oauth2
  • Updated UI text to be provider-agnostic when amber_api_url is not configured

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
app/models/user.rb Modified insufficient_credit method to include 'sofia_account' provider and changed and to && for better precedence
app/views/users/show.html.erb Made role description text conditional based on amber_api_url presence to support multiple authentication providers
public/images/alphalogonotextwhite.png Binary image file added (no review needed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/models/user.rb
Comment on lines 73 to 75
def insufficient_credit
provider == 'amber_oauth2' and credit.negative?
(provider == 'amber_oauth2' || provider == 'sofia_account') && credit.negative?
end
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The insufficient_credit method has been updated to include the 'sofia_account' provider, but there are no tests covering this new behavior. Consider adding test cases to verify that:

  1. Users with provider 'sofia_account' and negative credit return true
  2. Users with provider 'sofia_account' and positive credit return false
  3. Users with other providers and negative credit return false

This would ensure the credit warning functionality works correctly for sofia_account users.

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.68%. Comparing base (8599e24) to head (81c46d8).
⚠️ Report is 2 commits behind head on staging.

Additional details and impacted files
@@             Coverage Diff             @@
##           staging    #1167      +/-   ##
===========================================
+ Coverage    77.27%   77.68%   +0.41%     
===========================================
  Files           54       54              
  Lines         1351     1340      -11     
===========================================
- Hits          1044     1041       -3     
+ Misses         307      299       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lodewiges lodewiges merged commit 4b5a0f5 into staging Dec 11, 2025
6 checks passed
@lodewiges lodewiges deleted the feature/sofia-user-credit-warning branch December 11, 2025 17:33
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