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: urlencode filename for separate mod targets #47

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

Th3Fanbus
Copy link
Contributor

When a mod's name contains characters that should be urlencoded, any newly-uploaded versions cannot be download because of some errors regarding the URL being invalid. Make sure the returned key is urlencoded for consistency with the rest of the codebase.

@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 16.94%. Comparing base (867a34d) to head (f323a9f).

Files Patch % Lines
storage/storage.go 0.00% 5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           staging      #47      +/-   ##
===========================================
- Coverage    16.94%   16.94%   -0.01%     
===========================================
  Files          103      103              
  Lines         5440     5442       +2     
===========================================
  Hits           922      922              
- Misses        4417     4419       +2     
  Partials       101      101              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mircearoata
Copy link
Member

The linter is failing because of missing whitespace around the +.

Can you also fix the similar issue in RenameVersion that I mentioned in the other PR #44 (comment)?

return true, fmt.Sprintf("/mods/%s/%s.smod", modID, EncodeName(cleanName)+"-"+version)

@Th3Fanbus
Copy link
Contributor Author

Th3Fanbus commented Mar 6, 2024

The linter is failing because of missing whitespace around the +.

Can you also fix the similar issue in RenameVersion that I mentioned in the other PR #44 (comment)?

return true, fmt.Sprintf("/mods/%s/%s.smod", modID, EncodeName(cleanName)+"-"+version)

I think that should be it

EDIT: I ran gofumpt -e -d locally and force-pushed the commits, the CI should be happy now.

When a mod's name contains characters that should be urlencoded,
any newly-uploaded versions cannot be download because of some
errors regarding the URL being invalid. Make sure the returned key
is urlencoded for consistency with the rest of the codebase.

Signed-off-by: Angel Pons <th3fanbus@gmail.com>
As per Mircea's comment on [satisfactorymodding#44 (comment)](satisfactorymodding#44 (comment)):

    I've also now noticed that RenameVersion is also not correctly
    escaping the filename, it's only escaping the mod's name, and
    not the version, which is what causes mods that use build
    metadata in the version (+build.1234) to fail, so that one
    should need a similar fix of encoding the filename.

Signed-off-by: Angel Pons <th3fanbus@gmail.com>
@Vilsol Vilsol merged commit e284fcb into satisfactorymodding:staging Mar 7, 2024
3 checks passed
@Th3Fanbus Th3Fanbus deleted the patch-2 branch March 7, 2024 21:48
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.

4 participants