Review and accept/deny content syncs

[jmakowski1123]

When a component from a library has been updated in the library, the corresponding course block will display the "sync" icon.

When a user clicks on the sync icon, a modal pops up displaying a list of the updates.

Each update has an option to "view existing version" and "view new version". When either button is clicked, a view-only preview opens in a new tab.

Users have 3 options:

1. Accept the updates and override any local edits that may have been made to the block
2. Accept the updates while keeping any local edits that may have been made to the block
3. Deny the updates

[bradenmacdonald]

@kdmccormick Do we currently have a place to track if the user decides to deny an update (but keep the link)?

[kdmccormick]

@bradenmacdonald Nope, but I can add it to openedx/edx-platform#34925 easily enough.

I'm imagining another XBlock field, upstream_version_declined = Integer(...). When a user declines an update to version N, we set upstream_version_declined = N. That way, we know to show the update prompt again when the upstream library block's latest version is greater than upstream_version_declined.

Does that sound good?

[kdmccormick]

Assigning myself to finish the backend work for this, but I will need someone else to do the frontend work.

[bradenmacdonald]

@kdmccormick That works. And yep, we can do the frontend.

[bradenmacdonald]

@kdmccormick I'm thinking, if I implement this UI for any block with upstream + upstream_version, it should handle both the "pasted linked single XBlock" and the new Problem Bank / Random Pool XBlock, right? Because all the children of the Problem Bank will be treated like individual pasted linked single XBlocks, and so if any individual child has a newer version available, we can show that and allow syncing the child - there is not necessarily a need to implement a sync at the level of the Problem Bank block itself. At least for MVP. Does that make sense?

[kdmccormick]

@bradenmacdonald Yes!

[bradenmacdonald]

@jmakowski1123 Would something like this be OK for the "sync" button in the MVP (in the legacy UI)? It turns out it's a lot easier to out a nice-looking new button on the right than on the left, since the legacy UI is designed to have all actions on the right.

No update available:

When update available:

Mouse over:

[jmakowski1123]

That's fine for the MVP. We can shift it left when the unit MFE is implemented in Teak.

[jmakowski1123]

Looks good!

[bradenmacdonald]

@jmakowski1123 @kdmccormick I just realized a nuance here:

When previewing the changes, I can show the current version of the library content and I can show the new version, as specified in the description. But if the user has customized the block in the course (such as changing the title or the # of attempts), I can't show them a preview of how it would look after we update to the newest version but keep their customized [title].

Example:

Library version 1: Title is "Udnersatdnnig English Speling", content is "Speling is hard."
Author copies version 1 into Course A
Author copies version 1 into Course B.
Within Course B, the author changes the title to "Thinking about Spelling"
Library is updated to version 2: Title is "Understanding English Spelling", content is "Spelling is hard"

Now, in Course A there's no problem. We can show the preview and the author can see the old version 1 and the new version 2 and then confirm they want to accept the changes.

But in Course B, we can show the current version in the course (custom title "Thinking about Spelling", content is "Speling is hard.") and we can give them a preview of the new version 2 (Title is "Understanding English Spelling", content is "Spelling is hard"), but it won't exactly preview what they're about to see. Because when they accept it, they'll actually get a customized version 2 (Title is "Thinking about Spelling", content is "Spelling is hard").

I hope that's fine for MVP but maybe we should think about how to communicate it more clearly in future.

Edit: I guess if we always show the old library version (not the customized course version) and the new library version, it's a bit more clear.

[kdmccormick]

@bradenmacdonald Hm, good call out. If the preview rendering code is factored to accept an XBlock object instance, you could call sync_from_upstream(course_block) and then render course_block without committing the changes to modulestore. If the code isn't factored that way, then... let me think about it, maybe there's something easy we could do.

It would be a bummer if the Preview Changes feature didn't respect the new override behavior, which we've worked hard to make more predictable.

[bradenmacdonald]

@jmakowski1123 @kdmccormick @sdaitzman How is this looking?

@kdmccormick I realized that with this MVP implementation, you can't actually see the display name or num attempts anywhere so it doesn't matter (yet).

WIP.screen.recording.mov

[kdmccormick]

@bradenmacdonald I'll let Sam and Jenna comment on the UI, but it's exciting to see that working!!

To render the preview, are you using LMS's render_xblock view, or something else?

[bradenmacdonald]

@kdmccormick It's the new embed_block_view, which is what we're using to render previews of library components. In this case, it's showing the old and current published version of the library component, not a course XBlock.

[jmakowski1123]

Thanks @bradenmacdonald ! I think I'm ok with the new version/old version toggle UI for the MVP; it's a much better implementation than the very barebones one we initially had, which was to just open them in new tabs. Would like @sdaitzman thoughts?

What will the user see, for example, when there's a lot of text and the change is in the middle, or if there are multiple changes in different parts of the text body? With this small modal, I suspect there's not much optionality for presenting the changes in a visual way. For the MVP, I think it's ok to expect users can suss out the differences themselves, but if there are any very easy-to-implement visual cues, that would be good too. And maybe the modal should be bigger, similar to the editor modals?

[bradenmacdonald]

@jmakowski1123 For MVP, we won't be able to do any kind of highlighting of the differences. That will be a lot of work and need some design thinking as well as technical planning, so won't be possible for now. It will be up to users to "spot the differences". As for the modal, yes I can make it bigger though the only larger size seemed "too large" to me personally when I tried it out. You can let me know once you play with it on the sandbox (not yet ready).

[jmakowski1123]

Totally fine - this all works for MVP.

[bradenmacdonald]

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

[jmakowski1123]

I had to refresh my course outline page to trigger the update button to appear, is that the only feasible way to trigger it?

[jmakowski1123]

When I reuse the same component multiple times in a course, I have to manually accept the updates for each separate use of the component. I'm not sure we've actually discussed this use case. What's the current configuration? Is it even possible to sync the same change across multiple reuses of the same component in a particular course?

[kdmccormick]

@jmakowski1123 Ah, we did not consider that use case. Speaking from the backend implmentation, each reuse is handled separately. We could certainly build a bulk "sync all references of the same library item across a course" API for Teak, but I don't think we could fit that in before the Sumac freeze.

[jmakowski1123]

Definitely not a priority for Sumac. I'll create a ticket and we can figure out if it's a priority for Teak.

[bradenmacdonald]

Yep, we have a nice UI designed in Figma for syncing all the blocks in a course; it's just a question of when we implement it.

[bradenmacdonald]

I had to refresh my course outline page to trigger the update button to appear, is that the only feasible way to trigger it?

Yeah, it's necessary to refresh the page. Once we move to the MFE unit page perhaps we can get more "automagic" updating of the status on the page, but it's difficult to implement stuff like that in the legacy UI so a refresh is required for now.

[bradenmacdonald]

@jmakowski1123 Can we close out this issue?