-
Notifications
You must be signed in to change notification settings - Fork 1
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
Bug: "num components" not updated when a component in a collection is discarded #234
Comments
@ChrisChV Same issue when a new component is added to a collection -- can you cover that case here too? |
@pomegranited: Am I correct in understanding that the count of components for a Collection should differ whether we're looking at the draft or published view of things, i.e. whether we're editing the content in the library or browsing to bring content into a course? So we'd only count the publishable entities with a non-null draft version for the former, and non-null published version for the latter? |
@ormsbee Collections are only used in Studio, and I don't think there are any use cases for them in the LMS. So I don't think that the number of components should change based on their published state -- both draft and published components should always be counted here. And somehow, we need to exclude components "deleted" from the library too. |
@pomegranited Actually, there is a use case where only public components are shown: openedx/frontend-app-authoring#1420. When you want to add a library component to a course, the picker shows the library and its components in their published state. So yes, a wrong counter would be displayed if a collection has unpublished components. |
Collections aren't used by the LMS, but when you are adding a Library Component to a Course using the "Library Content (Beta)" button, you're taken to a modal view of the library that only shows published items, not draft ones. So if you have a Component that has never been published in that Collection, it shouldn't count towards the count. These are screenshots from that situation right now, where there's a Collection with one unpublished Component in it, and I'm adding it from a Course: Here the Collection is listed as having one item in it. ... but that item isn't published. So actually opening it shows as empty when using the "add to course" workflow. |
😛 Sorry, @ChrisChV's much more concise comment didn't show up for me until after I submitted mine. |
(and yeah, this was one of those blink-and-you'd-miss-it additions that happened in the flurry of merges last week 😛 ) |
I don't know exactly how the collection count is updated in Meilisearch, but is it okay if we say:
At that point I think soft-deletions, publishes, and reverts will all do the right thing? |
@ormsbee @ChrisChV Ahh.. sorry, I forgot about the picker!
Sure -- the "published" component count can sit with the other "published" fields. I don't know if a
Hoping so -- I think the issue with this ticket is making sure we update the index counts when soft-delete/publish/reverts happen. |
@pomegranited @ormsbee The problem is not the index, the index is updated when reverting. The problem is that if I revert a library, the collection still keeps the entities. If I add components and revert several times, the entities from the previous versions are still inside the collection, that's why the counter is never reduced (see video). My solution is to remove unpublished entities in collections when revert the library. |
I think it's okay if the I think that actually changing the membership of what PublishableEntities are in the Collection in response to reverts will lead us down a long path of playing wack-a-mole in different situations. |
For instance, if you delete an unpublished Draft that's in a Collection, and then discard those changes–then the Draft should re-appear in the Collection, right? Removing unpublished items from a collection at that point would do the wrong thing. |
Ok, now I understand better. Yes, I will implement this. Thanks! |
FYI @ormsbee @bradenmacdonald I used this new approach openedx/edx-platform#35734 |
@jmakowski1123 @lizc577 @sdaitzman @marcotuts This is ready for AC testing on the sandbox |
Steps to reproduce:
Expected behavior:
The only time we'd expect draft documents to be deleted is after calling "Discard changes" on a library with unpublished changes.
The text was updated successfully, but these errors were encountered: