Bug: "num components" not updated when a component in a collection is discarded

[pomegranited]

Steps to reproduce:

1. Create a library in the Authoring MFE
2. Add a collection to the library.
3. Add a component to the collection and do not publish.
4. Note that the "number of components" shown in the collection is "1".
5. View the library, and click "Discard changes" from the sidebar
6. The draft component will disappear from the library's and collection's components lists.
7. Note that the "number of components" for the collection card still says "1".

Expected behavior:

The only time we'd expect draft documents to be deleted is after calling "Discard changes" on a library with unpublished changes.

1. Create a library in the Authoring MFE
2. Add a collection to the library
3. Add a component to the collection and do not publish.
4. View the library, and click "Discard changes" from the sidebar
5. The draft component will disappear from the library's and collection's components lists.
6. The "number of components" for the collection card should be reduced from 1 to 0.

[pomegranited]

@ChrisChV Same issue when a new component is added to a collection -- can you cover that case here too?

[ormsbee]

@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?

[pomegranited]

@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.

[ChrisChV]

@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.

[ormsbee]

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.

[ormsbee]

😛 Sorry, @ChrisChV's much more concise comment didn't show up for me until after I submitted mine.

[ormsbee]

(and yeah, this was one of those blink-and-you'd-miss-it additions that happened in the flurry of merges last week 😛 )

[ormsbee]

I don't know exactly how the collection count is updated in Meilisearch, but is it okay if we say:

1. Reverts don't actually remove things from Collections, since Collections live outside the whole draft/publish workflow; but...
2. There are separate counts for "published components" and "draft components" that are used, which are derived from the counts where there exists a published version and there exists a draft version, respectively.

At that point I think soft-deletions, publishes, and reverts will all do the right thing?

[pomegranited]

@ormsbee @ChrisChV Ahh.. sorry, I forgot about the picker!

There are separate counts for "published components" and "draft components" that are used, which are derived from the counts where there exists a published version and there exists a draft version, respectively.

Sure -- the "published" component count can sit with the other "published" fields.

I don't know if a draft_count field is useful on its own, or if it's better to keep the "num_components" field that is draft + published counts -- up to @ChrisChV .

At that point I think soft-deletions, publishes, and reverts will all do the right thing?

Hoping so -- I think the issue with this ticket is making sure we update the index counts when soft-delete/publish/reverts happen.

[ChrisChV]

@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.

https://www.loom.com/share/effaaa2768ca44f59632aa59a62d66a9

[ormsbee]

I think it's okay if the PublishableEntity stays associated with the Collection. I think it's easier to reason about, because it keeps the Collections more ignorant of what happens during draft and publish operations–since Collections exist outside of that process. And it should give us the numbers we expect as long as we derive the counts based on non-null drafts in the Collection and the non-null published versions in the Collection, rather than just the count of PublishableEntities in the Collection.

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.

[ormsbee]

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.

[ChrisChV]

And it should give us the numbers we expect as long as we derive the counts based on non-null drafts in the Collection and the non-null published versions in the Collection, rather than just the count of PublishableEntities in the Collection.

Ok, now I understand better. Yes, I will implement this. Thanks!

[ChrisChV]

And it should give us the numbers we expect as long as we derive the counts based on non-null drafts in the Collection and the non-null published versions in the Collection, rather than just the count of PublishableEntities in the Collection.

FYI @ormsbee @bradenmacdonald I used this new approach openedx/edx-platform#35734

[ChrisChV]

@jmakowski1123 @lizc577 @sdaitzman @marcotuts This is ready for AC testing on the sandbox