Skip to content

Add shared comments for disbursements#13248

Draft
albertchae wants to merge 1 commit intomainfrom
commentable-disbursement
Draft

Add shared comments for disbursements#13248
albertchae wants to merge 1 commit intomainfrom
commentable-disbursement

Conversation

@albertchae
Copy link
Copy Markdown
Contributor

Summary of the problem

https://hackclub.slack.com/archives/C026RKHLPNJ/p1773163261567279

Previously, since we had a single HcbCode for disbursements, comments were always shared. Now they can be on either side of the disbursement, but we still want the ability to share comments selectively.

Describe your changes

Since both sides of a disbursement still have a connection to the Disbursement, we are adding Commentable to that model.

It was unavoidable to add shared comment specific methods to HcbCode but we tried to delegate them to models that make more sense as much as possible (e.g. shared_comment_recipient_label lives on ingoing/outgoing disbursements)

  • Switch from hardcoded redirect to redirect_back_or_to since it is exactly for the case we want to handle. Redirect to the original commentable as a fallback because we don't want to redirect to a Disbursement on a shared comment vs the HcbCode that the comment is originating from.
  • Add all_comments to Commentable that aggregates comments from both the primary commentable and its shared_commentable. Currently it doesn't own ordering because that's already handled in the view, but we should probably push that to the model
  • UI shows all comments on your side and shared comments, interleaved by created_at (post time). For the checkbox we show the name of the other. Added some styling to shared comments similar to admin comments. side, but for created comments we can't since they are only stored on the shared Disbursement model and it's possible the author has access to both sides of the disbursement.
  • Minor refactor of HcbCode#disbursement? to use existing predicate methods

Copy link
Copy Markdown
Member

@garyhtou garyhtou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's another approach we can try:

  • Add a select dropdown on the frontend.
    • Can be implemented as a dropdown button (GitHub style)
  • Depending on the selected option, it will set a few form fields:
    • commentable_type and commentable_id
    • return_to (this is where the controller redirects after the POST)
      • In practice, this will just be an HcbCode URL (the url of the page which the form is being rendered)
  • controller would only need to be updated to handle return_to
    • redirect_back_or_to url_from(return_to) || @commentable

Comment on lines +59 to +61
elsif record.commentable.is_a?(Disbursement)
# TODO: possibly replace this by adding #events to Disbursement?
user_list = record.commentable.source_event.users + record.commentable.destination_event.users
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's implement Disbursement#events so that it uses the first case in this if statement and includes ancestor_users

Comment on lines 50 to 51
user_list = record.commentable.events.collect(&:users).flatten
user_list = record.commentable.events.collect(&:ancestor_users).flatten
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is buggy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check whether ancestor_users already includes users

Comment thread app/models/disbursement/outgoing.rb Outdated
Copy link
Copy Markdown
Member

@garyhtou garyhtou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback from call:

  • Remove red styling for "shared comments". Shared comments shouldn't affect the comment card border.
  • Keep the "shared with X" on the top right of the comment card, but polish up the styling
  • On the Disbursement show page, list comments from all three commentables (Disbursement, Outgoing Disbursement, Incoming Disbursement)
  • On the Disbursement show page, the "Add comment" form adds the comment to the Disbursement commentable. The "Add comment" button should make it extremely clear that you're adding to both sides.
    • We will not add the option to select a commentable right now (no choice between Disbursement, Outgoing Disbursement, Incoming Disbursement). The only option for commentable is the Disbursement.
  • On each transaction, the other transaction contains comments, which means there are comments not visible on the current page. Add a link to view all comments.
    • Example: Outgoing disbursement has 2 comments, Disbursement has 3 comments, Incoming disbursement has 4 comments. On the outgoing disbursement show page it says "View 4 other comments" and links to the Disbursement show page. On the incoming disbursement, it says "View 2 other comments" and links to the disbursement show page.

@albertchae albertchae force-pushed the commentable-disbursement branch from ff2c3ad to eb085e9 Compare April 17, 2026 19:04
@albertchae
Copy link
Copy Markdown
Contributor Author

  • fix showing the shared with label across incoming/outgoing vs the main disbursement
  • move view other side link to below bottom comment and above text box

@albertchae albertchae force-pushed the commentable-disbursement branch 2 times, most recently from 9516477 to 95ddf09 Compare April 20, 2026 05:48
@hackclub hackclub deleted a comment from github-actions Bot Apr 20, 2026
@hackclub hackclub deleted a comment from github-actions Bot Apr 20, 2026
@hackclub hackclub deleted a comment from github-actions Bot Apr 20, 2026
@albertchae albertchae force-pushed the commentable-disbursement branch 10 times, most recently from 5a2cff3 to 9abd2cd Compare April 20, 2026 06:52
Comment thread app/views/hcb_codes/show.html.erb Outdated
Comment on lines +53 to +56
@hcb_code.outgoing_disbursement.disbursement.incoming_disbursement.local_hcb_code
else
@hcb_code.incoming_disbursement.disbursement.outgoing_disbursement.local_hcb_code
end %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@hcb_code.outgoing_disbursement.disbursement.incoming_disbursement.local_hcb_code
else
@hcb_code.incoming_disbursement.disbursement.outgoing_disbursement.local_hcb_code
end %>
@hcb_code.outgoing_disbursement.disbursement.incoming_disbursement.local_hcb_code
else
@hcb_code.incoming_disbursement.disbursement.outgoing_disbursement.local_hcb_code
end %>

@albertchae albertchae force-pushed the commentable-disbursement branch 4 times, most recently from 135407f to ad9a210 Compare April 20, 2026 07:11
@hackclub hackclub deleted a comment from github-actions Bot Apr 20, 2026
Previously, since we had a single HcbCode for disbursements, comments
were always shared. Now they can be on either side of the disbursement,
but we still want the ability to share comments selectively.

Since both sides of a disbursement still have a connection to the
Disbursement, we are adding Commentable to that model.

It was unavoidable to add shared comment specific methods to HcbCode but
we tried to delegate them to models that make more sense as much as
possible (e.g. `shared_comment_recipient_label` lives on
ingoing/outgoing disbursements)


- Switch from hardcoded redirect to `redirect_back_or_to` since it is
  exactly for the case we want to handle. Redirect to the original
  commentable as a fallback because we don't want to redirect to a
  Disbursement on a shared comment vs the HcbCode that the comment is
  originating from.
- Add `all_comments` to Commentable that aggregates comments from
  both the primary commentable and its shared_commentable. Currently it
  doesn't own ordering because that's already handled in the view, but
  we should probably push that to the model
- UI shows all comments on your side and shared comments, interleaved by
  created_at (post time). For the checkbox we show the name of the
  other. Added some styling to shared comments similar to admin
  comments.
  side, but for created comments we can't since they are only stored on
  the shared Disbursement model and it's possible the author has access to
  both sides of the disbursement.
- Minor refactor of `HcbCode#disbursement?` to use existing predicate methods

dropdown button

PR feedback

show all comments on disbursement

no indication for non shared comments

mentionable?

⏺ The @mention suggestions in the comment textarea. When you type @, it shows a dropdown of users you can mention. The comment_mentionable method returns which users appear in that list.

  For HcbCode, it includes users from comments, events, and point of contacts. For Disbursement, it currently returns an empty array (the default), so no suggestions will appear.

add annotation on disbursements page

link to other side comments
@albertchae albertchae force-pushed the commentable-disbursement branch from ad9a210 to 184f368 Compare April 20, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants