Learn more about these different git repos.
Other Git URLs
Adding reactions to Issues / PRs can fail if the url contains a href anchor or just any other additional parameters.
Works: https://pagure.io/pagure/pull-request/5240 becomes: https://pagure.io/pagure/pull-request/5240/comment/182129/react
https://pagure.io/pagure/pull-request/5240
https://pagure.io/pagure/pull-request/5240/comment/182129/react
Fails: https://pagure.io/pagure/pull-request/5240?error=true becomes: https://pagure.io/pagure/pull-request/5240?error=true/comment/182129/react expected: https://pagure.io/pagure/pull-request/5240/comment/182129/react
https://pagure.io/pagure/pull-request/5240?error=true
https://pagure.io/pagure/pull-request/5240?error=true/comment/182129/react
https://pagure.io/pagure/issue/5340# becomes: https://pagure.io/pagure/issue/5340#/comment/835331/react expected: https://pagure.io/pagure/issue/5340/comment/835331/react
https://pagure.io/pagure/issue/5340#
https://pagure.io/pagure/issue/5340#/comment/835331/react
https://pagure.io/pagure/issue/5340/comment/835331/react
https://pagure.io/pagure/issue/5340#comment-835331 becomes: https://pagure.io/pagure/issue/5340#comment-835331/comment/835331/react expected: https://pagure.io/pagure/issue/5340/comment/835331/react
https://pagure.io/pagure/issue/5340#comment-835331
https://pagure.io/pagure/issue/5340#comment-835331/comment/835331/react
Error
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN"> <title>405 Method Not Allowed</title> <h1>Method Not Allowed</h1> <p>The method is not allowed for the requested URL.</p>
Affected file: https://pagure.io/pagure/blob/master/f/pagure/static/reactions.js#_3 Line 3 set the url variable for the POST request but uses the plain location.href value without validation / filtering.
location.href
Not discovered by the unit test because only valid requests are send, one time authenticated, one time without authentication. A valid authentication with a wrong URL like above isn't covered.
Unit Tests: https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_fork.py#_7738 https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_fork.py#_7770
https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_issues.py#_5760 https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_issues.py#_5794
Metadata Update from @wombelix: - Issue tagged with: UI, bug, unit-tests
Possible Solutions
1) Adding the type (Issue/PR) and (ID) as data attribute, for example data-type and data-id, to the reaction picker div: https://pagure.io/pagure/blob/master/f/pagure/templates/_formhelper.html#_149 https://pagure.io/pagure/blob/master/f/pagure/templates/_formhelper.html#_214 Adjusting https://pagure.io/pagure/blob/master/f/pagure/static/reactions.js to read those additional values and generating the URL based on these data instead of location.href.
data-type
data-id
2) Continue to use location.href in https://pagure.io/pagure/blob/master/f/pagure/static/reactions.js but adding a some filtering into the javascript part to remove href anchors and parameters from the url.
Metadata Update from @wombelix: - Issue assigned to wombelix - Issue set to the milestone: 5.13
Metadata Update from @wombelix: - Assignee reset
Metadata Update from @wombelix: - Issue set to the milestone: 6.x (was: 5.13)
Log in to comment on this ticket.