General updates

[lodewiges]

Doing 2 different updates

Removing the orders_total colum because it is not used #1079

Fixing the configuration of Sentry

Switched to database consisty #984

update node version in dockerfile #996

Summary by CodeRabbit

• Chores

  • Update error-tracking to set the current user ID as a JSON value for events.
  • Replace a development/test consistency gem with an updated alternative and adjust CI comments referencing it.
  • Upgrade the Node.js runtime used in builds and add additional lint steps to CI (JS and Sass).

• Database Changes

  • Remove the order_total column from orders and bump the DB schema version.

[coderabbitai]

Walkthrough

Replaces a Sentry scope callback in the application layout with a direct Sentry.setUser call using current_user&.id.to_json; removes the order_total decimal column from orders via a migration and updates schema version; updates Docker Node setup, swaps a gem, and adds CI lint steps.

Changes

Cohort / File(s)	Summary
Sentry user assignment app/views/layouts/application.html.erb	Replaced Sentry.configureScope callback that set the user with a direct Sentry.setUser call using current_user&.id.to_json embedded in the layout JS.
DB migration: remove order_total db/migrate/20251026202155_remove_order_total_from_orders.rb, db/schema.rb	Added migration RemoveOrderTotalFromOrders removing :order_total (decimal) from orders; updated db/schema.rb version and removed the order_total column definition.
Node version (Docker) Dockerfile	Updated Node.js setup from setup_16.x to setup_20.x.
Gem swap Gemfile	Replaced consistency_fail with database_consistency, ~> 2.0.6 in the :development, :test group.
CI lint updates bin/ci.sh	Updated commented DB consistency reference from consistency_fail → database_consistency; added yarn lint and yarn run sass-lint -v -q steps alongside yarn install.

Sequence Diagram(s)

sequenceDiagram
  participant Server
  participant Browser
  participant SentryJS as Sentry (browser)

  Note over Server,Browser: Old layout (embedded JS)
  Server->>Browser: HTML with JS: Sentry.configureScope(callback => Sentry.setUser(...))
  Browser->>SentryJS: configureScope invokes callback -> setUser executed

  Note over Server,Browser: New layout (embedded JS)
  Server->>Browser: HTML with JS: Sentry.setUser({ id: <json> })
  Browser->>SentryJS: setUser called directly

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Review migration for intended data removal and rollback path.
• Verify current_user&.id.to_json is properly escaped/embedded for JS.
• Check compatibility and any config changes for database_consistency.
• Confirm Docker Node upgrade and new yarn lint steps work in CI.

Poem

🐇
I hopped through lines and nudged a trace,
Set one small user in its place.
An order's number left the plain,
New node and gem — refreshed the lane.
Code garden tidy, I munch again.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description does not follow the required template structure provided by the repository. The description is missing the mandatory Checklist section entirely, including the critical database migration checklist items that are particularly important given this PR includes a database migration. Additionally, the description lacks the organized Summary and Other information sections specified in the template. While the description does identify all major changes and reference related issues, the issues are not formatted in the proper 'fixes #xyz' syntax. The informal, unstructured format and absence of template compliance make it difficult for reviewers to verify that required steps have been completed.	Please restructure the pull request description to follow the template format. Ensure the Checklist section is completed with confirmations about merging database migrations and testing the database migration steps as specified. Organize the changes under a clear Summary section and use the proper 'fixes #xyz' syntax when referencing related issues. Move any additional context to the Other information section. This ensures reviewers can quickly verify that all necessary steps have been completed before merging.
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.
Title Check	❓ Inconclusive	The pull request title "General updates" is vague and generic, using a non-descriptive term that doesn't convey meaningful information about the actual changes in the PR. While the changeset contains multiple distinct updates—including removal of the order_total column, Sentry configuration fixes, switching dependency management tools, upgrading Node.js, and adding yarn lint steps—the title fails to highlight or summarize any of these primary changes. A teammate scanning the commit history would not understand what this PR actually accomplishes based on this title alone.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch GeneralUpdates

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5c6b14 and 57ebc06.

⛔ Files ignored due to path filters (1)

• Gemfile.lock is excluded by !**/*.lock

📒 Files selected for processing (1)

• Gemfile (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• Gemfile

⏰ 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). (1)

• GitHub Check: Build

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

app/views/layouts/application.html.erb (1)

34-34: Consider using .to_json for safer Ruby-to-JavaScript interpolation.

While the current approach works (user IDs are integers and the 'null' string outputs correctly as a JavaScript keyword), using .to_json is a more robust and idiomatic approach for interpolating Ruby values into JavaScript contexts.

Apply this diff:

-        Sentry.setUser({ id: <%= current_user&.id || 'null' %> });
+        Sentry.setUser({ id: <%= current_user&.id.to_json %> });

This handles the nil case automatically (outputs JavaScript null) and provides proper escaping for any edge cases.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between baf8f54 and b95a05a.

📒 Files selected for processing (2)

• app/views/layouts/application.html.erb (1 hunks)
• db/migrate/20251026202155_remove_order_total_from_orders.rb (1 hunks)

🔇 Additional comments (1)

db/migrate/20251026202155_remove_order_total_from_orders.rb (1)

1-5: Migration is safe but missing column attributes for perfect reversibility.

The column removal is safe because all codebase references to order_total work through the method (defined in app/models/order.rb:24-26), not direct column access. The method will continue calculating from order_rows after the column is removed. JSON serialization (orders_controller.rb), views (activities/show.html.erb), and JavaScript all call this method and will function correctly.

For complete reversibility, consider updating the migration to include the column attributes:

remove_column :orders, :order_total, :decimal, precision: 8, scale: 2

This matches the current schema definition (db/schema.rb:84) and ensures a clean rollback if needed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.56%. Comparing base (baf8f54) to head (57ebc06).
⚠️ Report is 1 commits behind head on staging.

Additional details and impacted files

@@           Coverage Diff            @@
##           staging    #1099   +/-   ##
========================================
  Coverage    74.56%   74.56%           
========================================
  Files           51       51           
  Lines         1093     1093           
========================================
  Hits           815      815           
  Misses         278      278           

☔ 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.