[openstreetmap/openstreetmap-website] Move changeset comments feed to resourceful routes (PR #4587)

Andy Allan notifications at github.com
Wed Mar 20 15:08:42 UTC 2024


@gravitystorm requested changes on this pull request.

This feels like a "one step forward, one step back" PR to me unfortunately.

There's two parts in moving to resourceful routing, one part is using `resources` declaration in the routing file, but the other part is trying to have resourceful controller methods i.e. just index/show/new/create/edit/update/destroy , which the resources declarations use by default.

In #2050 (specifically https://github.com/openstreetmap/openstreetmap-website/commit/4b0d56f7e15d2929c973265e3b545e69273b8cb5) I renamed the `comments_feed` (which is what was used when the code was in changesets_controller) to the `index` method, since that's what most closely aligns with what this doing i.e. showing a list of objects. So I'd like to keep this as a method named `index`.

I realise that we still have `feed` methods elsewhere, but those are targets for refactoring too. There's a difficulty when there are already `index` methods that do different things (e.g. different lists of objects, or search filters, or other things, so in those cases it can't be as simple as just a `respond_to` switch) but in those cases I would prefer to see e.g. `FooController::index` and `Foo::FeedController::index` or similar, i.e. the method name is still `index`.

See http://jeromedalbert.com/how-dhh-organizes-his-rails-controllers/ for more info.



-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4587#pullrequestreview-1949180161
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4587/review/1949180161 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240320/a5c3c842/attachment.htm>


More information about the rails-dev mailing list