Unclear Message When Trying to Merge Works

[JordanFrederick]

Problem

I continually received the message, "The string did not match the expected pattern" when first trying to merge records. The meaning of the message was unclear to me, leading me to believe I was doing something (and causing confusion). Perhaps this message could be altered so it is more clear.

Evidence / Screenshot

Relevant URL(s)

Reproducing the bug

1. Go to perform a merge as a super librarian outside of the API group.
2. Do the merge

• Expected behavior: The error should say that the account has the wrong permissions.
• Actual behavior: There is either a JavaScript error from JSON.parse, or something (probably JavaScript?) saying "The string did not match the expected pattern".

Context

• Browser (Chrome, Safari, Firefox, etc):
• OS (Windows, Mac, etc):
• Logged in (Y/N): Y
• Environment (prod, dev, local): prod

Notes from this Issue's Lead

Proposal & constraints

Related files

Stakeholders

---

Instructions for Contributors

• Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.

Project Breakdown

As a threshold matter, I (@scottbarnes) would add that the fix for this may be trivial, but the setup is a bit involved. Additionally, I failed to replicate this exactly, but I (likely) got close with the following error when trying to merge: JSON.parse: unexpected character at line 1 column 1 of the JSON data.

The context here is that when someone is added to /usergroup/super-librarians, they still can't execute merges in the merge queue, and instead also need /usergroup/api permissions.

This issue is concerned merely with the display of the error message. A super librarian without API group permissions should get an error about the account not having the correct permissions.

Here's the set up to reproduce a version of this error:

• Get the Docker local development environment going. Docker instructions.
• Add a new user to be your super librarian test user: https://github.com/internetarchive/openlibrary/wiki/Creating-and-Logging-in-as-a-new-user-on-your-local-client. Follow that through step 2, but skip step 3. Pay attention to the username you select.
• Log in the local host as [email protected]
• Go to http://localhost:8080/usergroup/super-librarians?m=edit. You'll see /people/openlibrary as a member. In another empty cell, add /people/<the username you created>, and save the page.
• Go to http://localhost:8080/admin/people, click the user you created, click the button to activate the account (even though you already did that via the console).
• From that same menu, click "login as this user".
• Now go try to perform a merge. Follow these directions. You may want to add some more works via http://localhost:8080/books/add. They can take a few minutes to show up in the search. You can also just search for book or even * and pick some books you never even knew were there: http://localhost:8080/search?q=*&mode=everything
• Once you click "merge" you should see some error text. In my case, this said JSON.parse: unexpected character at line 1 column 1 of the JSON data.
• You can now begin working on a fix. Checking out the JavaScript files in this output related to merging may or may not be a good place to start: "git grep JSON.parse" to see some of those.
• You may also need to apply this fix for author merging.

[schu96]

Hi @scottbarnes, I was able to replicate the steps in the project breakdown and can submit a PR that changes the error message to inform a user that they may not have the correct permissions to merge works.

However I was not able to access the merge authors page with just super librarian privileges, is another role that is required to access that page?

[scottbarnes]

@schu96, you may need /usergroup/librarians privileges also. I am unsure whether it's better to update the code that checks the permissions so the merge button only displays when someone has the appropriate privileges, or whether it's best to show the error and include the missing usergroup(s) in the error message.

I think for now lets stick with showing an error that details the missing privileges, on the theory this way it's more obvious which permissions are missing.

[schu96]

@scottbarnes

I took a glance at the merging authors page + relevant files and it seems that there is already a way to check that there are valid user permissions to determine if a user can access the page in merge_authors.py, though I am not entirely sure if that is the case.

I was able to merge authors with an account that only has librarian and super librarian permissions, and verified that accounts without these specific permissions would not have access to the merge authors page.

For now, I will push my PR that outputs a clearer error message for merging works.

[scottbarnes]

Thanks for this, @schu96. I will attempt to review this Thursday, though it may not happen until Friday.