Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 500 when saving /lists/add with URL parameters + fix global lists editing #8463

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

cdrini
Copy link
Collaborator

@cdrini cdrini commented Oct 27, 2023

Closes #8245 . Closes #8246 .

Fix. Saving from the /lists/add endpoint was erroring when used with url parameters to pre-populate the form. This was because these url parameters would be forwarded along with the POST request when the form was submitted (since the action was unset on the form). The endpoint now checks the request body independently/separately (by default, web.input() combines data from both).

Fix. Global lists (eg /lists/OL1L) were erroring when edited due to the ?m=edit/edit rewrite rule not triggering, causing the endpoint to be handled by infogami instead of our custom code. Fixed the rewrite rule.

Fix. The /edit rewrite rule was erroring for users when a list had unicode characters in its url (see this users unfortunate list description!). Use the safe_seeother that performs url encoding.

Technical

Testing

Tested locally:

  • Create global list without url params
  • Create global list with url params
  • Edit global list
  • Add emoji to list name + edit list
  • Create personal list via lists/add
  • Edit personal list

Screenshot

Stakeholders

@davidscotson

When no action is specified on the form, it forwards along all the url
parameters. This was causing it trouble since it was having collisions between
what was specified in the url parameters, vs what was specified in the
form data.

This commit makes it explicitly consider the form data separately first,
so that will always take precedence.
@cdrini cdrini changed the title Fix/lists add save Fix to /lists/add API and global lists Oct 27, 2023
@cdrini cdrini changed the title Fix to /lists/add API and global lists Fix 500 when saving /lists/add with URL parameters + fix global lists editing Oct 27, 2023
By default, a <button> acts as a submit button, so will be triggered on
enter.
@cdrini cdrini marked this pull request as ready for review October 27, 2023 08:33
@cdrini cdrini added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Oct 27, 2023
@mekarpeles mekarpeles merged commit ae9feb4 into internetarchive:master Oct 27, 2023
3 checks passed
@mekarpeles mekarpeles self-assigned this Oct 27, 2023
@cdrini cdrini deleted the fix/lists-add-save branch October 27, 2023 18:50
@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow creation of lists via hand-crafted links of seed items
3 participants