rubyforgood / rubyforgood/awbw

Retire /people/:id/all_comments now that the combined feed replaces it

Open
#2,368 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
15
Forks
26
Avg merge
12h 42m
Merged PRs (30d)
242

Description

🤖 From Claude: Filed as a follow-up to #2355.

What

/people/:id/all_comments is no longer linked from anywhere. #2355 added /people/:id/comments_and_communications — a superset that shows the same aggregated comments interleaved with the person's communications — and repointed every caller at it:

  • the combined comments & communications section on the person, event registration, scholarship, story, and story idea forms
  • topic_subscriptions/_form.html.erb
  • continuing_education_registrations/edit.html.erb
  • users/_form.html.erb

The old page is still reachable by typing the URL, but nothing navigates there.

Why it wasn't done in #2355

The instruction was to repoint the links, not to delete the page — and the two pages aren't quite identical, so retiring it is a judgement call worth making deliberately:

  • The new page covers it. Both use PersonCommentAggregator, both have a composer with the same record picker, and the new one adds communications, more filters, and full-height bodies.
  • What differs. all_comments edits a comment inline (comments/_aggregated_comment + comments/create.turbo_stream.erb prepending into the feed). The combined page is read-only per row — you click through to the record to edit. If inline editing matters, port it before deleting.

Scope if we retire it

  • config/routes.rb — the get :all_comments member route
  • PeopleController#all_comments and its entry in the set_person before_action
  • app/views/people/all_comments.html.erb, app/views/people/person_comments_results.html.erb
  • spec/requests/people_all_comments_spec.rb
  • the "Aggregated comments for a person" entry in config/features.yml (or repoint its action_path at the new page)
  • check whether comments/_feed and comments/_aggregated_comment still have callers — the global /comments index uses them, so they stay

Alternative

Keep it addressable and leave it unlinked. Cheap, but it becomes a second person-comments page that drifts.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with config/routes.rb, PeopleController, and the listed all_comments views, then read spec/requests/people_all_comments_spec.rb and the feature entry in config/features.yml. Confirm whether inline editing must be preserved, then verify the obsolete route and callers are retired or repointed without removing shared comments partials still used by /comments.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
backend, testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.