Apache Allura™

Forge software for hosting software projects

Brought to you by: alexluberg, brondsem, deshani, dill0wn, and 10 others

Git Merge Request #317: Improved reactions list by adding interactivity (merged)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Shalitha Suranga wants to merge 9 commits from /u/shalithasuranga/allura/ to master, 2019-03-15

Second fix for [#8263]

Commit	Date
[fef3fe] (patch13) by Shalitha Suranga [#8263] Revert markdown permission check	2019-03-15 10:12:30	Tree
[32a2b8] by Shalitha Suranga [#8263] Change permission check in comment view	2019-03-15 03:35:11	Tree
[63bef8] by Shalitha Suranga [#8263] Limit reactions list interactivity using permission checks	2019-03-14 03:56:19	Tree
[1a54fe] by Shalitha Suranga [#8263] Interactive reaction list and source formatting	2019-03-11 05:41:49	Tree
[2fd2c7] by Shalitha Suranga Merge branch 'master' of https://gitbox.apache.org/repos/asf/allura	2019-03-11 02:35:41	Tree
[793e91] by Shalitha Suranga Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/allura	2019-01-14 10:14:40	Tree
[e874b1] by Shalitha Suranga Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/allura	2018-11-27 07:40:28	Tree
[c6a6ab] by Shalitha Suranga Merge branch 'master' of https://github.com/shalithasuranga/allura	2018-11-27 07:39:58	Tree
[4f7fba] by Shalitha Suranga , pushed by GitHub Merge pull request #1 from apache/master Taking latest changes	2018-10-05 04:33:18	Tree

Discussion

Dave Brondsema - 2019-03-13

Looking pretty good! I did notice that if you aren't logged in and don't have permission to add a reaction, the hover cursor looks like you can click on it. And you actually can, but the AJAX method goes to an invalid URL that ends up not doing much. Could we avoid all that if the user doesn't have permission?

👍
1

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Shalitha Suranga - 2019-03-14

Hi.. Dave

Thanks for the review 👍. Yes I've added the permission check now. Please check

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2019-03-14

The CSS class is a good way to handle it, but I think the permission needs to be changed. Not many people will have 'moderate' access. Looks like we just check against is_anonymous on the reaction-button element, and on the post_reaction endpoint. So it should match that.

👍
1

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Shalitha Suranga - 2019-03-15

Yeah agree. endpoint checks for is_anonymous permission. So I changed the permission check now please check

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2019-03-15

Status: open --> merged
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2019-03-15

Thanks!

🎉
1

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Log in to post a comment.