Merchants: Display all merchants on Settings page

[DeathCamel58]

Noticed that on the Merchants settings page, only merchants that are used on at least one transaction are displayed. This makes it impossible to remove or edit a merchant if it's unused on a transaction.

This page now lists all merchants.

Summary by CodeRabbit

• Bug Fixes
  • Updated the family merchants list to pull merchants from the correct family association so users now see the full and accurate set of merchants for their family. Alphabetical ordering and display behavior remain unchanged, preserving the existing presentation and sort order.

[coderabbitai]

Walkthrough

Index action in the family merchants controller now reads merchants from Current.family.merchants.alphabetically instead of Current.family.assigned_merchants.alphabetically; rendering and ordering remain unchanged.

Changes

Cohort / File(s)	Summary
Merchant association change app/controllers/family_merchants_controller.rb	Switched index action to use Current.family.merchants.alphabetically instead of Current.family.assigned_merchants.alphabetically; updated accompanying comment to "Show all merchants for this family".

Sequence Diagram(s)

(omitted — change is a single-association swap with no control-flow impact)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Files to spot-check: app/controllers/family_merchants_controller.rb (verify association semantics and any authorization/decorator expectations).

Possibly related PRs

• Merchants fixes #262 — modifies the same controller index line and switches between merchants and assigned_merchants, potentially overlapping or conflicting.

Suggested reviewers

• jjmata

Poem

🐰 I swapped a branch with gentle cheer,
From assigned to all the merchants near,
A tiny hop, a clearer view,
One line changed — a brighter crew! ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly matches the main change: displaying all merchants instead of only assigned merchants on the Settings page.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 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 c49832a and ce73459.

📒 Files selected for processing (1)

• app/controllers/family_merchants_controller.rb (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

app/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/

Files:

• app/controllers/family_merchants_controller.rb

**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules

Files:

• app/controllers/family_merchants_controller.rb

**/*.{rb,js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.

Files:

• app/controllers/family_merchants_controller.rb

app/{models,controllers,views}/**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Avoid N+1 queries

Files:

• app/controllers/family_merchants_controller.rb

🔇 Additional comments (1)

app/controllers/family_merchants_controller.rb (1)

7-8: LGTM! Change correctly implements the PR objective with proper inheritance handling.

The switch from assigned_merchants to merchants now displays all merchants on the Settings page. Since FamilyMerchant inherits from Merchant, it properly inherits the alphabetically scope and the code will work correctly. This allows editing or removing unused merchants as intended.

---

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: 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 8860450 and c49832a.

📒 Files selected for processing (1)

• app/controllers/family_merchants_controller.rb (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

app/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/

Files:

• app/controllers/family_merchants_controller.rb

**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules

Files:

• app/controllers/family_merchants_controller.rb

**/*.{rb,js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.

Files:

• app/controllers/family_merchants_controller.rb

app/{models,controllers,views}/**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Avoid N+1 queries

Files:

• app/controllers/family_merchants_controller.rb

🔇 Additional comments (1)

app/controllers/family_merchants_controller.rb (1)

8-8: I need to verify the Family model's associations and check for N+1 query concerns. Let me run additional verification:

I need to verify the Family model definition and check line 48 for comparison:

Let me search the codebase more directly for the Family model and the controller code:Based on the previous script execution output and my web research, I can now provide a comprehensive analysis. The original script confirmed that the alphabetically scope exists on the Merchant model (line 10) with only order(:name). However, the ast-grep pattern didn't find the Family model's merchants association - this could mean either the association doesn't exist, or the pattern didn't match it properly.

Let me generate the final rewritten review comment based on the verification performed:

Verify the merchants association on the Family model and check for N+1 query prevention.

The change from assigned_merchants to merchants aligns with the PR objectives to display all merchants. To prevent N+1 queries with associations, ensure the merchants collection is eager loaded using includes, preload, or eager_load—with includes being the recommended default choice.

The alphabetically scope on the Merchant model only orders by name and doesn't include preloading. Verify:

1. That the Family model actually defines has_many :merchants
2. Whether merchants are accessed in a loop within the view or controller for this action
3. Whether .includes(:merchants) is applied to the query in the controller or through a default scope

[jjmata]

Since you just touched that code, can you review @sokie?

[sokie]

I think we should allow both FamilyMerchant and ProviderMerchant support in the settings page, because it's weird to have merchants in the db and not available in the UI.
But I also see that adding ProviderMerchant needs more extensive Controller changes.
Good catch and we'll merge this!