Flow/Thanks

Flow Thanks is a feature to allow users to thank others for their comments on Flow discussion boards. It was developed by Facebook Open Academy students. This page serves both to identify issues and document ideas and progress. Trello card

Progress report

edit

As of 3 April

edit
  • Done:
  • In progress:
    • Code review for the special page patches
  • To do:
    • Investigate mediawiki-vagrant performance
      • Try figuring out how to get HHVM to work

As of 27 March

edit

As of 20 March

edit
  • Done:
  • In progress:
  • To do:
    • Special:Flow redirector

As of 13 March

edit

As of 6 March

edit
  • Done:
    • Back button handling (in the form of showing an alert on repeated thanks)
    • i18n description strings
  • In progress:
  • To do:
    • Implement no-JavaScript fallback

As of 1 March

edit
  • Done:
    • Submitted patches for code review
    • PHPUnit integration tests for Flow
  • In progress
    • PHPUnit tests for Thanks
    • Browser tests
  • To do
    • Handle back button after thanking (currently the state is not preserved)
    • i18n description strings
    • Implement no-JavaScript fallback

As of 25 February

edit
  • Done:
    • Initial version done, not yet submitted to Gerrit
  • In progress / to do:
    • Submit initial version for code review
    • Create browser tests
    • Implement no-JavaScript fallback

Issues

edit
  • Figure out if we can re-use Extension:Thanks
    • Thanks stores states in session and cookies for last 100 thanks made
    • Thanks link for edits appears on a history page, but according to trello, Flow has it besides the post itself.
    • Thanks depends on revId, Flow stores its own id.
  • getCreatorId on local wikis changed: earlier posts return a string, newer return an integer. As a result code confused about which posts were written by someone else and are thankable.
  • Security:
    • Sanitizing all inputs
    • Right now AbstractRevision::getContent() is used to obtain topic title; method docs say we should use Templating::getContent() which does permission checks--is this necessary? Fixed
    • The API should check whether a post ID is valid I think this is done in getFlowData()
  • Supporting users with JavaScript disabled
    • Right now the link links to a non-existent special page
  • Testing! JavaScript unit tests and browser tests
  • Remembering Thanked state (Thanks does it in PHP session and cookie)
    • Ensure the cookie and session functionality to remember Thanked state does the right thing
  • Validate assumptions (thanking original poster vs. editor, etc.)
  • Other refinements
  • Make sure emailed notifications work properly
    • Can't test this in mediawiki-vagrant
    • Tested on ee-flow. Seems to work.

Things spage noticed

edit
  • I wound up with two thanks-thanked cookies, one for the path /wiki/index.php and one for /wXXXXpath That might be a Thanks issue.
  • After I did a Flow thanks, I had a thanks-thanked cookie with no value.
  • In the Thanks.flow onFlowAddPostInteractionLinks hook, $rev isn't a Revision object, it's a FlowRevision.

What Thanks does

edit
  • Register a new type of notifications with Echo
  • Eventually calls ApiThank::sendThanks(), which creates an Echo event
  • Handles Echo's hooks to format and display notifications

Existing implementations

edit

Normal page Thanks implementation

edit

Thanks responds to HistoryRevisionTools and PageHistoryBeforeList hooks,

  • responds in insertThankLink by adding HTML <a href ...>Thank</a>

no-JavaScript: link to https://www.mediawiki.org/wiki/Special:Thanks/677745

JS: jQuery dialog, upon confirm make API call https://www.mediawiki.org/w/api.php?action=thank&format=json&rev=677745&source=diff&token=1aabf45aa2917fd61527b2941ad19304%2B%5C , response is success: 1, recipient: "username"

Recipient sees

Thanker thanked you for your edit on Page

Thanker links to person, the text links to https://www.mediawiki.org/w/index.php?title=Page&oldid=prev&diff=677745

Normal Echo notification of a Flow post

edit

A Flow mention generates:

User mentioned you in their post Text of post on "Talk:Main Page".

Submitting patch

edit
# basically git pull origin master to keep master up to date
$ git checkout master
$ git fetch origin
$ git merge origin/master  

# rebasing
$ git checkout flow-thanks
$ git checkout -b my-patch # keep the reference to flow-thanks, in case we screw up later. Give it a better name, see gerrit branch naming convention.
$ git rebase -i master  # squash all commits into a single one, see also http://git-scm.com/book/en/Git-Tools-Rewriting-History the squashing commit part

# prepare to submit patch
$ git commit --amend # make sure the commit message is proper
$ git review -R # submit without rebasing against master

Modifying the patch

edit

After we've submitted the first patch, I recommend we only use the old branch for new patch sets of that original patch. For other purposes we start new branches (be it tests, or new features).