Page MenuHomePhabricator

Don't make it seem user can re-thank for a Flow post when they already did; render server-side
Open, MediumPublic

Description

The only reason T101340: mw.thanks.getUserGender should deduplicate requests is even an issue is because when Flow renders posts, it always renders them with "Thank" links, and then Thanks JS comes in and replaces the ones where you've already thanked for that post with "Thanked" non-links. However, the knowledge that I've already thanked this user doesn't appear to be obtained from the API, it must be embedded in the page somehow. Presumably that means we should be able to render the correct message immediately rather than having JS fix it up?


Other bug summary from T134621 (with have nice video)

thank_not_persist.gif (900×1 px, 2 MB)

The screenshot is an illustration of the following scenario:

  1. On any Flow board click to Thank some users in one post - all clicked 'thanks' will change to 'thanked'.
  2. In the same post click on any Reply and after typing in the post, save the post.
  3. All previously displayed 'thanked'' will changed to 'thank'.

Event Timeline

Catrope raised the priority of this task from to Needs Triage.
Catrope updated the task description. (Show Details)
Catrope subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

There are two separate ways this is tracked:

  1. Session data (see userAlreadySentThanksForId in ApiFlowThank).
  2. Cookie (flow-thanked).

If we really want to refactor this and do it 100% properly, these are both wrong. Session data and cookie data both expire. We need something persistent on the server like a DB table; then the server can render it correctly (per this bug) (side note, server already tries to do this right for ordinary revision thanks, but it uses the session which will eventually expire).

The client doesn't even really need to cache it then, AFAICT: After AJAX, just update the DOM to de-link and say 'Thanked', then on next page reload, the server will render it.

Catrope triaged this task as Medium priority.Jul 1 2015, 11:35 PM
Catrope moved this task from Untriaged to Near-Term Interest on the Collaboration-Team-Triage board.

Steps to reproduce the issue:

  1. On any Flow board click to 'Thank' anyone.
  1. 'Thank' changes to 'Thanked' and a proper notification is sent.
  1. Click to change the sorting order of a board - 'Thanked' changes to 'Thank again'.
  1. Click that reverted 'Thank' - the following page will be displayed:

JS in the Console:

[FLOW] Failed to getInstanceByElement: no $container.length

[FLOW] Error in component handler: makeContentInteractive TypeError: component.emitWithReturn is not a function

Screen Shot 2015-11-16 at 11.47.29 AM.png (235×584 px, 24 KB)

  1. 'Send thanks' can be clicked again - no new notifications will be sent which is good.
  2. Stay on the Special page-Send thanks page and log out.
  3. Log in as a user to whom Thank was sent - the user upon login will be on the page.
  4. Click on 'Send thanks' - 'Thank action failed. Please try again.'

Screen Shot 2015-11-16 at 12.29.40 PM.png (292×614 px, 29 KB)

Mattflaschen-WMF renamed this task from Thanked-ness for Flow posts should be rendered server-side, not client-side to Don't make it seem user can re-thank for a post when they already did; render server-side.Jul 5 2016, 8:06 PM
Mattflaschen-WMF renamed this task from Don't make it seem user can re-thank for a post when they already did; render server-side to Don't make it seem user can re-thank for a Flow post when they already did; render server-side.
Mattflaschen-WMF added a subscriber: Zppix.