Merge Queue tweaks

[Eds-Dbug]

Closes #

a.) Removes top of page pagination.
b.) Removes the "Show requests that I've reviewed" and "Show my requests" at the top of the page.
c.) Makes the make the MR title a link to preview the merge without assigning.

Technical

Changes in merge_request_table.html only

Testing

Mostly visual. Will update next section with images.

Screenshot

Will update

Stakeholders

@mekarpeles @jimchamp

[jimchamp]

Thanks @Eds-Dbug. I've added some review notes.

Adding a link to the merge page is a little more involved than just nesting the title in an anchor element, for a couple of reasons:

1. When unauthenticated, the author and work merge pages return 404s and 403s, respectively. I didn't check, but I suspect that the same will be true for authenticated patrons who are not members of either the librarians, super-librarians, or admin usergroups.
2. mrid is necessary to close merge requests. If a super-librarian clicks a title link, and then clicks the merge button, the corresponding merge request will not be closed. Worse still, resubmitting a work or author merge has unknown, but potentially very harmful, effects.

Item 1 is easy enough to fix, but item 2 will likely require some community discussion to come up with a good solution.

Go ahead and remove that link for now.

[jimchamp]

Suggested change

from math import ceil

Remove unused import.

[jimchamp]

Suggested change

	$:_('<span class="mr-details__request-title"><a href="%(url)s" target="_blank">%(title)s</a></span>', title=request_title, url=request["url"])
	<span class="mr-details__request-title">$request_title</span>

Remove link. Render span itself --- there's nothing to translate, so no need to use gettext here.

[Eds-Dbug]

Noted ill do the fix and hop on a communal discussion.

…

On Tue, Dec 12, 2023 at 8:28 PM jimchamp ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks @Eds-Dbug <https://github.com/Eds-Dbug>. I've added some review notes. Adding a link to the merge page is a little more involved than just nesting the title in an anchor element, for a couple of reasons: 1. When unauthenticated, the author and work merge pages return 404s and 403s, respectively. I didn't check, but I suspect that the same will be true for authenticated patrons who are not members of either the librarians, super-librarians, or admin usergroups. 2. mrid is necessary to close merge requests. If a super-librarian clicks a title link, and then clicks the merge button, the corresponding merge request will not be closed. Worse still, resubmitting a work or author merge has unknown, but potentially very harmful, effects. Item 1 is easy enough to fix, but item 2 will likely require some community discussion to come up with a good solution. Go ahead and remove that link for now. ------------------------------ In openlibrary/plugins/upstream/edits.py <#8591 (comment)> : > @@ -5,6 +5,7 @@ import web from openlibrary import accounts +from math import ceil ⬇️ Suggested change -from math import ceil Remove unused import. ------------------------------ In openlibrary/templates/merge_request_table/table_row.html <#8591 (comment)> : > @@ -26,7 +26,7 @@ $# End : Status dot indicator <div class="mr-details"> - $:_('<span class="mr-details__request-title">%(title)s</span>', title=request_title) + $:_('<span class="mr-details__request-title"><a href="%(url)s" target="_blank">%(title)s</a></span>', title=request_title, url=request["url"]) ⬇️ Suggested change - $:_('<span class="mr-details__request-title"><a href="%(url)s" target="_blank">%(title)s</a></span>', title=request_title, url=request["url"]) + <span class="mr-details__request-title">$request_title</span> Remove link. Render span itself --- there's nothing to translate, so no need to use gettext here. — Reply to this email directly, view it on GitHub <#8591 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AT4EQGCMGE2JNFHYX7KJCGLYJEAFNAVCNFSM6AAAAABAIB4B5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONZYGY2TMMBZHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jimchamp]

Lgtm. Thanks, @Eds-Dbug.