#7685 Subscribe/unsubscribe action should use POST

v1.3.1
closed
General
Heith Seewald
2015-08-10
2014-09-16
No

Currently all of subscribe/unsubscribe buttons (in the topbar of any tool's page and in the wiki sidebar) are using GET to make an action. Their should require POST to avoid CSRF.

See also discussion at [#4905]

Related

Tickets: #4905
Tickets: #7944

Discussion

  • Dave Brondsema

    Dave Brondsema - 2015-06-01

    An additional bug in the current GET links: currently forum discussion pages don't enforce a trailing slash on the URL so if you go to /p/myproject/discussion/help the sub/unsub link doesn't work but it does from /p/myproject/discussion/help/

     
  • Dave Brondsema

    Dave Brondsema - 2015-06-15
    • labels: --> sf-current, sf-2
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-06-16
    • Owner: Anonymous --> Igor Bondarenko
    • Labels: sf-current, sf-2 --> 42cc, sf-current, sf-2
    • Status: open --> in-progress
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-03
    • status: in-progress --> review
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-03

    Closed #802. ib/7685

     
  • Heith Seewald

    Heith Seewald - 2015-07-08
    • Reviewer: Heith Seewald
     
  • Heith Seewald

    Heith Seewald - 2015-07-09
    • status: review --> in-progress
     
  • Heith Seewald

    Heith Seewald - 2015-07-09

    This is looking good.

    One small UX improvement would be to use $.ajax to submit the post so it doesn't need to pull down the whole page again.

     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-13
    • status: in-progress --> review
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-13

    Closed #819. Force-pushed ib/7685

    I changed it to ajax, but there are still some improvements needed that I wanted to discuss with you beforehand.

    Firstly, in allura theme we don't have separate icons for "subscribed" and "unsubscribed" state. So it seems to user that nothing changes, when she clicks "Subscribe/unsubscribe" button.

    SF theme does not have this problem, since we're using font for icons and "active" styles actually work there (blue text-shadow).

    I think we should use Font Awesome or something instead of sprites on allura theme also. Probably will need another ticket for that. Or at least have separate sprites for different states.

    Secondly, there're no any feedback when subscribe request is in progress, i.e. immediately after click. I have tried to use a little spinner for that, but there are gradient on the background and a spinner with transparent background looks ugly on such scale. Do you have some ideas what we can use to indicate request progress? Maybe some css transition to scale or spin the icon?

    Thirdly, we have one confusing use case for subscriptions. Let's suppose user is subscribed to entire tracker and then he tries to subscribe to the individual ticket in this tracker. He will not be subscribed to the ticket, since he already has subscription for entire tool, so even if we would fix my first and second point above, user will see that nothing changed, and that's also confusing. I'm thinking we should show some kind of message in that case to clear things up. It could be tooltip-like message above the subscription icon or something like flash message in the top right corner. What do you think?

     
  • Heith Seewald

    Heith Seewald - 2015-07-16

    These are great notes :)

    • I created a ticket to address the updated icons: [#7924].
    • Maybe could use a combination of “cursor: wait;” and/or the msg notify system to indicate request progress/completion?
    • If someone is already subscribed to the parent of the current item, I think it would make more sense to have a "subscribed to {parent}" type label or something in place of the button.

    I like the idea of a tooltip-like message that makes that even clearer. But yeah, I agree that we probably shouldn’t allow an ‘unsubscribe’ button if they’re already subscribed to the parent -- that might get confusing.

     

    Related

    Tickets: #7924

  • Heith Seewald

    Heith Seewald - 2015-07-16
    • status: review --> in-progress
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-17

    "cursor:wait" seems like a good idea, thanks! I'm not sure about "suscribed to {parent}" label. It will clear things up, but maybe it will be too long for the toolbar. Let's try it, though.

     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-22
    • status: in-progress --> review
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-07-22

    Closed #825. Force-pushed ib/7685 (rebase)

    Added "cursor: wait" + notification system based on tooltipster plugin, so that user will see clearly what's going on.

     
  • Heith Seewald

    Heith Seewald - 2015-07-22
    • status: review --> closed
     
  • Heith Seewald

    Heith Seewald - 2015-07-22

    This looks great Igor.

    Do you think we should change this into a 4 with the additional work you did?

    Merged/Closed

     
    • Igor Bondarenko

      Igor Bondarenko - 2015-07-22

      I think so. This definitely feels like more than 2.

       
  • Dave Brondsema

    Dave Brondsema - 2015-07-27
    • labels: 42cc, sf-current, sf-2 --> 42cc, sf-2
     
  • Dave Brondsema

    Dave Brondsema - 2015-08-10
    • Milestone: unreleased --> v1.3.1
     

Log in to post a comment.