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.

Sign up for GitHub

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

Jump to bottom

fix(package upload): content of uploaded packages is now not permanently stored in the system #289

Merged
merged 8 commits into from
Dec 3, 2019
Merged

fix(package upload): content of uploaded packages is now not permanently stored in the system #289

merged 8 commits into from
Dec 3, 2019

Conversation

sr258
Copy link
Member

@sr258 sr258 commented Nov 26, 2019 •
edited
Loading

  • change signatures of PackageImporter to return metadata and parameters instead of contentId
  • implement logic to copy content files to temporary storage

@sr258 sr258 added this to the Milestone 1 milestone Nov 26, 2019
@sr258 sr258 added the [status] in progress PRs that are currently being worked on. label Nov 26, 2019
@sr258
Copy link
Member Author

sr258 commented Nov 30, 2019

Turns out the POST request to the 'filter' Ajax endpoint invokes some method of H5PContentValidator in the PHP implementation, which is a 1340 lines long class doing lots of stuff...

I don't want to implement this right away, but have a closer look at what this is doing. At the moment, I simply don't "filter" the parameters but return them. Uploading packages works fine, though, but maybe things aren't as safe as we want them to be.

@sr258 sr258 added [status] needs review PRs that are ready to be reviewed. and removed [status] in progress PRs that are currently being worked on. labels Nov 30, 2019
@sr258
Copy link
Member Author

sr258 commented Nov 30, 2019

Closes #278

@sr258 sr258 marked this pull request as ready for review November 30, 2019 20:31
@sr258
Copy link
Member Author

sr258 commented Dec 2, 2019 •
edited
Loading

Rerunning the tests worked... weird.

@JPSchellenberg JPSchellenberg merged commit f5d84ca into master Dec 3, 2019
@JPSchellenberg
Copy link
Member

🎉 This PR is included in version 0.10.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sr258 sr258 deleted the fix/temporary-upload-package branch December 3, 2019 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JPSchellenberg JPSchellenberg JPSchellenberg approved these changes

Assignees
No one assigned
Labels
released [status] needs review PRs that are ready to be reviewed.
Projects
None yet
Milestone
Milestone 1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sr258 @JPSchellenberg