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

Display only published content and published titles/descriptions/previews when browsing content to insert into a course #1354

Open
Tracked by #1095
bradenmacdonald opened this issue Oct 1, 2024 · 8 comments · Fixed by #1534

Comments

@bradenmacdonald
Copy link
Contributor

bradenmacdonald commented Oct 1, 2024

When users are in a course and open the modal to pick library content to insert into their course (either as regular linked content or as part of a randomized problem bank), we want them to be browsing the published version of the library, not the draft. But this is a challenge because our search index currently only has the draft version.

Screenshot 2024-10-01 at 2 45 43 PM

Screenshot 2024-10-01 at 2 46 14 PM

^ The above view should show the published version.

Notes

The studio search index has always only included "draft" content because the original use case (searching course content in Studio) never needed to search published content. And most of the new libraries features don't need that either - except this one workflow.

We can easily exclude "never published" content from the search results already. The problem is displaying the old version of content that was published and then changed, but those changes haven't been published yet.

We can always show the correct (published) version in the Preview by requesting the backend render the published version 1. The problem is more the search results.

Suggested solution

Add display_name_published and description_published as new fields in the studio index. When (re-)indexing a library component, fill those in with the last published name and/or description.

Note: currently the "description" is generated on the frontend from content but we should generate description and description_published on the backend that same way.

Then, we can update the frontend to show either the draft or the published version in search results depending on the context.

Post-sumac we may also need a problem_types_published to avoid an edge case with capa problems that change type, but we can fix that later.

We don't have to store this data for the course blocks in the index; only library content.

As an optimization, when we know that the published version is the same as the draft version, don't bother "properly generating" the _published variants (e.g. don't call index_dictionary() on the published version) - just set them equal to the draft version immediately (or leave them blank - see below).

Open Questions

Should we store display_name_published and description_published in all cases, or only when they're different from the draft display_name/description? Doing the latter might make the index slightly smaller and searches slightly faster, even though it makes displaying the published version of the cards more complicated.

How will this work with keyword search? Is it possible to search specific fields in the "normal" (draft) view, and search other fields in the "published" view? i.e. normally we search display_name+description, but sometimes we search display_name_published+description_published.

Footnotes

  1. Though technically this isn't fully implemented yet

@bradenmacdonald
Copy link
Contributor Author

@ormsbee @kdmccormick @pomegranited Does this plan make sense for how to deal with the need to sometimes show a search results view that only shows published stuff?

@pomegranited
Copy link
Contributor

@bradenmacdonald

Sure, this approach seems fine for this.. are there any other use cases of this data?

Add display_name_published and description_published as new fields in the studio index. When (re-)indexing a library component, fill those in with the last published name and/or description.

What about searching the published html/capa content itself? We can store a "published" version of that content field too.

Will the frontend also need to know the full published usage key (including the published version), so we know which version this linked component is referencing?

So maybe it would be clearer if we nest these fields together like we do with tags? e.g.

   "published": {
        "display_name": ...,
        "description": ....,
        "content": ...,
        "usage_key": "<usage key with published version>",
   }

Should we store display_name_published and description_published in all cases, or only when they're different from the draft display_name/description?

I'd rather always store these fields even if they're the same... but I won't die on this hill 😄

@bradenmacdonald
Copy link
Contributor Author

What about searching the published html/capa content itself? We can store a "published" version of that content field too.

I'm proposing having that data summarized in the description[_published] field by the backend during indexing, and we don't currently have any other use for content. So for now I wouldn't bother.

Will the frontend also need to know the full published usage key (including the published version), so we know which version this linked component is referencing?

No. For v2 libraries, I designed the opaque keys so that they don't have version or branch fields. In my opinion, having several different keys for the same piece of content depending on the context made the modulestore code very messy and was responsible for a variety of bugs. For v2 stuff, we're specifying draft vs. published vs. specific versions via separate parameters, not as part of the key.

So maybe it would be clearer if we nest these fields together like we do with tags? e.g.

Up to whoever implements this, but I think that looks nice.

@bradenmacdonald bradenmacdonald changed the title Store "published title" and "published description" in studio search index Display only published content and published titles/descriptions/previews when browsing content to insert into a course Oct 21, 2024
@bradenmacdonald bradenmacdonald moved this to Ready for AC testing in Libraries Overhaul Oct 22, 2024
@bradenmacdonald
Copy link
Contributor Author

@jmakowski1123 @sdaitzman This is ready for acceptance testing on the sandbox.

@ChrisChV
Copy link
Contributor

@jmakowski1123 @sdaitzman friendly reminder that this is ready for a AC testing on the sandbox

@sdaitzman
Copy link

Looks good to me overall, one minor issue I noticed. The OLX source within the component's sidebar under Details → Advanced details → OLX Source shows unpublished changes. (Separately, I still do think that menu should be hidden behind a URL parameter or other developer/advanced-user configuration option, but that's a separate issue)

image image

@ChrisChV
Copy link
Contributor

ChrisChV commented Dec 6, 2024

The OLX source within the component's sidebar under Details → Advanced details → OLX Source shows unpublished changes

@jmakowski1123 @sdaitzman It's fixed and ready for AC testing on the sandbox

@sdaitzman
Copy link

Thanks, @ChrisChV! The unpublished-draft OLX no longer shows up for me.

image

I did notice that the Component History still shows a "last modified" date that is more recent than the "last published" date. We should really upgrade that history to show "last modified" separately when "last published" as applicable, but I'm okay with this behavior for now since the Manage Tab provides some clarity on the unpublished draft changes.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for AC testing
Development

Successfully merging a pull request may close this issue.

4 participants