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

WebHost: Fix too-many-players error not showing #4033

Merged

Conversation

black-sliver
Copy link
Member

What is this fixing or adding?

"generating of multiworlds is limited to X players" error is currently not showing because the code path does not return a response.
This change is fixing it by using a redirect back to the form, which then shows the error.

Also adds a test for that and 2 more simple tests to validate basic functionality of the generate endpoint.

How was this tested?

  • Uploading a single yaml with 21 players and seeing the error.
  • Added tests.

@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Oct 5, 2024
@black-sliver black-sliver added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Oct 5, 2024
@black-sliver black-sliver force-pushed the fix-webhost-too-many-players branch from 76e3fbc to 5cf6f82 Compare October 5, 2024 12:14
WebHostLib/generate.py Fixed Show fixed Hide fixed
@black-sliver black-sliver force-pushed the fix-webhost-too-many-players branch from 5cf6f82 to 93032e7 Compare October 5, 2024 12:19
@black-sliver
Copy link
Member Author

black-sliver commented Oct 5, 2024

Anyone has an idea how to write this redirect without CodeQL complaining?
Technically it is safe because we redirect the user to the same url again

Think i got it

Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with a local webhost, the changes LGTM and the tests pass.

@black-sliver black-sliver merged commit 6287bc2 into ArchipelagoMW:main Oct 5, 2024
18 checks passed
@black-sliver black-sliver deleted the fix-webhost-too-many-players branch October 5, 2024 16:14
DestinyPlayer added a commit to DestinyPlayer/Archipelago-stellaris-world that referenced this pull request Oct 7, 2024
WebHost: Fix too-many-players error not showing (ArchipelagoMW#4033)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants