Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Load and manage products from a library project #1005
Load and manage products from a library project #1005
Changes from 4 commits
aaadaff
e0ff639
dcb838e
735409f
0bec953
574ea35
8965a88
0866c00
004e962
3078ba2
d9c1a29
ec56335
7a22491
a70135b
77e5317
e625901
7edc759
ef26dc2
7935ed3
b25715e
a30698e
562f2ed
eea31b6
e21c7a1
87907b5
b28f4b0
47fa5e5
fafcbe8
10e66c4
5d7aeaf
1776df1
5c115ce
2eb97b9
1ab7a65
9a0e490
51ec36e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the wrong approach because we'd need to know which product id belongs to which project. What @iLLiCiTiT mentioined before is to separate the query to
self._controller.get_version_items()
to have separate calls per project. So above, we'd get the product ids per project and then here we would do:As stated before - as far as I know on the backend nothing is keeping the database from having a product entity with the same
id
across different projects. Is that correct @martastain ? (It's unlikely by just creating products to happen, but when cloning e.g. projects I suppose it may very well share the same ids?) As such, we should still make sure to be explicit about which project each entry belongs to.The same goes for
version_items_by_product_id
dict. That should technically also be "per project" because otherwiseproduct_id
may overlap accidentally.@iLLiCiTiT thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funnily enough - this now doesn't need to be "per project" anymore - because the full method operates within the project's context. (by having a
project_name
input)Only as soon as you touch the variables on the instance itself
self
then you'd need to ensure to access them for the relevant project.If it makes more sense, we could also just change those global cache dicts by moving them into one per project cache.
And then we could do:
If that makes more sense to you. But that may just be overkill.
Anyway, it all boils down to: whenever we cache something to the entity id - we should do it by the hash of
(project_name, entity_id)
otherwise it's not unique.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can do this in separate PR as enhancement.
Not sure if there would be lots of changes in regard to this implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should implement the support correctly. It should not be that hard, it requires a lot of changes in code, but in practice it means we just have to consider project name as one hierarchy level above everything else (everywhere). If you struggle to achieve that, let me know.