[admin] Embed SolidusAdmin routes in SolidusAdmin components

[elia]

Summary

SolidusAdmin routes should be available by default.
Probably will require a last minute rebase to account for any newly merged PR.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• 🛣️ I have opened a PR to update the guides.
• ✅ I have added automated tests to cover my changes.
• 📸 I have attached screenshots to demo visual changes.

[codecov]

Codecov Report

Merging #5504 (28803d8) into main (eab114b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #5504   +/-   ##
=======================================
  Coverage   88.93%   88.94%           
=======================================
  Files         622      622           
  Lines       14918    14921    +3     
=======================================
+ Hits        13268    13271    +3     
  Misses       1650     1650           

Files	Coverage Δ
...min/app/components/solidus_admin/base_component.rb	95.45% <100.00%> (+0.71%)	⬆️
...mponents/solidus_admin/products/index/component.rb	91.17% <ø> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

[tvdeyen]

I think we should not mix in the route helpers as regular methods, but use a prefix instead (ie routes).

[tvdeyen]

I think I would prefer a method routes, so that we can a) extend it and b) easily separate the routes from regular methods in the components.

[tvdeyen]

It would be easier to understand where this method comes from by prefixing it with routes, like with helpers.

Suggested change

	account_path: account_path,
	account_path: routes.account_path,

[elia]

@tvdeyen I see the point, the current implementation is based purely on the existing VC implementation that includes them by default. That said, as a counterpoint, I would offer that routes already are separated by using exclusively the _path and _url suffixes, it's not as hard as a separation as using something like routes but serves the purpose of not mixing them with other methods nonetheless.

[tvdeyen]

The VC folks made helpers available with the helpers for very good reasons. They are not part of the current component scope. A routes proxy would follow that pattern and will make it much easier to extend or replace them by other extensions. Magically mix methods into the current view scope is what people disliked about helpers in Rails and is something that VC explicitly want to avoid. We should not violate that pattern.

[elia]

@tvdeyen I was referring to this https://github.com/ViewComponent/view_component/blob/main/lib/view_component/base.rb#L477-L481.

Routes along with some helpers[1] are included in the base, the only reason we initially went with separate routes helpers was to mimic Rails having engine scopes. But we're working from within the engine, and if anything would make sense to have the engine routes immediately available and the host app routes available through main_app. The other reason was tied to the migration from the legacy backend, and for that we need to easily identify the legacy routes in order to ensure they're completely absent before the final release.

---

1: VC::Base inherits from AV::Base which includes AV::Helpers which in turn includes a fair amount of generic helpers https://github.com/rails/rails/blob/7-1-stable/actionview/lib/action_view/helpers.rb

[tvdeyen]

But we're working from within the engine

We might be working from another engine (extensions) or even the main Rails app. If this is still possible, ok. But I think the interface would be much easier if we can inherit from a Solidus component and simply override or extend the routes method.

[tvdeyen]

Maybe you could provide a reason where you see the advantage in mixing this as module instead of using a proxy method. Is there a bug you want to fix? You just proved a lot of reasons for avoiding the proxy method. Where are the pros of the module mix in? Can you give an example?

[elia]

@tvdeyen it's quite simply less overhead and noise, i.e. removing the redundancy of typing solidus_admin. from within components inheriting from SolidusAdmin::BaseComponent.

[tvdeyen]

I do not agree with this, tbh. Less magic, extensibility and explicitness is way more important, imo. Especially in open source projects.