THREESCALE-9973: Bullet unused eager loading offences [Not Ready for Review]#4282
Open
Madnialihussain wants to merge 11 commits intomasterfrom
Open
THREESCALE-9973: Bullet unused eager loading offences [Not Ready for Review]#4282Madnialihussain wants to merge 11 commits intomasterfrom
Madnialihussain wants to merge 11 commits intomasterfrom
Conversation
…ry line_items sum
…ser include and obsolete entries
Madnialihussain
changed the title
Madnialihussain
changed the title
Madnialihussain
changed the title
…numeric type consistency
❌ 3 blocking issues (4 total)
|
Contributor
|
Your work looks great! Could you please stop extending this PR and file new ones. It will become impossible to review if many exceptions are handled at the same time. It is already too big with 21 files. Ideally we would have 1 removed exception per PR. I'm looking at this PR now. |
…ding false positive
akostadinov
reviewed
Apr 22, 2026
| Bullet.add_safelist class_name: "Cinstance", type: :unused_eager_loading, association: :user_account | ||
| Bullet.add_safelist class_name: "Invoice", type: :counter_cache, association: :payment_transactions | ||
| Bullet.add_safelist class_name: "Invoice", type: :unused_eager_loading, association: :line_items | ||
| Bullet.add_safelist class_name: "Invoice", type: :unused_eager_loading, association: :provider_account |
Contributor
There was a problem hiding this comment.
How did you verify these were false positives?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Fixes all unused_eager_loading offences in the Bullet safelist. The safelist is reduced from ~50 unused_eager_loading entries down to 3 intentional ones (Bullet's limitation with polymorphic_url).
Which issue(s) this PR fixes:
https://issues.redhat.com/browse/THREESCALE-9973
Verification steps:
Notes for your reviewer:
32a1a0c: Fix ApiDocs::Service :service false positive, test setup used service: nil, updated to use a real service
7f3546e: Move application_plans include to plans_for_filter and make pricing_rules conditional on finance.allowed?
431a175: Remove unused [:original] from bought_plans, Plan#xml_builder never accesses it
a6abedc: Remove .includes(:issuer) from ApplicationPlansController#collection, only plan.issuer_id (a column) is ever read
18739a7: Remove plan: [:service] from ApplicationPlanLimitsController, plan is already in memory via Rails inverse association
9656bfc: Decouple application finder from applications collection in BuyersApplicationsController, single-record actions were inheriting collection-level includes unnecessarily
86f5457: Fix Invoice unused eager loading, remove buyer_account, decouple invoice finders, use cached line_items sum instead of SQL aggregate
6d38f08: Make admin_user include conditional on buyer context, remove obsolete ServicePlan :pricing_rules entry, deduplicate Metric entries