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

#3015 - Add test for single file collections not working properly when copying media from another model. #3025

Closed
wants to merge 2 commits into from

Conversation

victor-priceputu
Copy link

Added test to replicate issue presented in #3015

…ly when copying media from another model.
@freekmurze
Copy link
Member

That is indeed failing, could you also add the fix to this PR?

@victor-priceputu
Copy link
Author

I will try to make an efficient fix. I am thinking of checking if we can take the max from the already-loaded media collection (if possible) and in case we cannot, to make the DB call for the sort, to try and find an optimal solution.

will have to make some local testing with bigger collections to see which way is better rounded for more cases when it comes to the amount of data

@patinthehat
Copy link
Collaborator

@victor-priceputu Are you still able to work on adding a fix for this issue? If you don't have time right now no problem - I can look into implementing a fix in a separate PR if necessary. Thanks!

@victor-priceputu
Copy link
Author

@patinthehat unfortunately the quickest I can work on it properly is in one month.
I just updated my fork and I cannot make the fix in just a few minutes I have available at the moment (there are some tests failing for me and I have to check them out as well)

@patinthehat
Copy link
Collaborator

@victor-priceputu Thanks for the update. Please post a comment once you've submitted a PR or let us know if you don't have time and we'll create an issue for this that someone else can pick up. Thanks!

@victor-priceputu
Copy link
Author

Hey, I tried to do this over the holiday but had no time, and do not see any spare time any time soon. If anyone wants to pick this up, please feel free to do so with my thanks

@freekmurze
Copy link
Member

Closing due to inactivity.

@freekmurze freekmurze closed this Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants