-
Notifications
You must be signed in to change notification settings - Fork 269
Review submission side panel #5630
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
base: unstable
Are you sure you want to change the base?
Review submission side panel #5630
Conversation
|
👋 Thanks for contributing! We will assign a reviewer within the next two weeks. In the meantime, please ensure that:
We'll be in touch! 😊 |
|
Hi @Jakoma02 - great to hear from you again! |
AlexVelezLl
left a comment
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.
Thanks a lot @Jakoma02! It's a pleasure to have you around here again! This is looking great! The side panel is working as expected, and you nailed the design! Just asked some refactors on a couple of things, but nothing major. Thank you!
| const { | ||
| isLoading: latestSubmissionIsLoading, | ||
| isFinished: latestSubmissionIsFinished, | ||
| data: latestSubmissionData, | ||
| fetchData: fetchLatestSubmission, | ||
| } = useLatestCommunityLibrarySubmission({ channelId: props.channel.id, admin: true }); |
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.
Could we add a submissionId prop to this component so that we can render any submission using this side panel? This will be useful to render this on the submission history page too!
| const versionPublishedData = computed(() => { | ||
| if (!latestSubmissionIsFinished.value) return undefined; | ||
| const publishedData = props.channel.published_data; | ||
| return publishedData[latestSubmissionData.value.channel_version]; | ||
| }); |
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 now have a Django model to store information about individual channel versions! https://github.com/Jakoma02/studio/blob/5008c1ec7a23b2a2857cffd39c9fc6683380c192/contentcuration/contentcuration/models.py#L1411. Could you create a small viewset for it in the viewsets/channel.py module with just two filters channel and version so that we can load the version info here? Thanks!
(Note: Now that we have this model, we could have a related field on the CommunityLibrarySubmission model instead of having the channel and version fields, but we will handle this in a future issu.e)
| watch(statusChoice, (newChoice, oldChoice) => { | ||
| if (!oldChoice) { | ||
| // The status was changed because the initial loading of the submission | ||
| // data completed. In that case, the flag reason choice has already been | ||
| // pre-filled from the submission data, so we do not want to overwrite it. | ||
| return; | ||
| } | ||
| flagReasonChoice.value = flagReasonChoiceOptions[0]; | ||
| }); |
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 we can just skip the reset of the flagReasonChoice ref, there is no problem if the selection remains once they select the flagged status back
| <span class="detail-annotation">Status</span> | ||
| <CommunityLibraryStatusChip | ||
| v-if="latestSubmissionIsFinished" | ||
| :status="latestSubmissionData.status" |
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 here we should use the uiSubmissionStatus too, right?
| function updateStatusInStore(newStatus) { | ||
| return store.commit('channel/UPDATE_CHANNEL', { | ||
| id: props.channel.id, | ||
| latest_community_library_submission_status: newStatus, |
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.
great!
| .then(async () => { | ||
| const snackbarText = | ||
| statusChoice.value === CommunityLibraryStatus.APPROVED | ||
| ? 'Submission approved' | ||
| : 'Submission flagged for review'; | ||
| showSnackbar({ text: snackbarText }); | ||
| updateStatusInStore(statusChoice.value); | ||
| }) | ||
| .catch(async () => { | ||
| showSnackbar({ text: 'Changing channel status failed' }); | ||
| }) | ||
| .finally(() => { | ||
| currentlySubmitting.value = false; | ||
| }); |
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.
Could we use async/await here to make the code a bit more readable? Thanks!
| const boxTitleColor = computed(() => paletteTheme.grey.v_800); | ||
| onMounted(async () => { | ||
| await fetchLatestSubmission(); |
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.
Here, we don't actually need to wait until the component is mounted to load the submission; we can just call the fetchLatestSubmission method directly on the body of the setup function.
| .wide-textbox { | ||
| width: 100%; | ||
| } | ||
| .wide-textbox ::v-deep .textbox { | ||
| max-width: 100%; | ||
| } |
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 now have an appearanceOverrides prop on KTextbox! We can use that instead!
| fetchCollectionAsAdmin(params) { | ||
| return client | ||
| .get(window.Urls.adminCommunityLibrarySubmissionList(), { params }) | ||
| .then(response => { | ||
| return response.data || []; | ||
| }); | ||
| }, | ||
| create(params) { | ||
| return client.post(this.collectionUrl(), params).then(response => { | ||
| return response.data; | ||
| }); | ||
| }, | ||
| resolveAsAdmin(id, params) { | ||
| return client | ||
| .post(window.Urls.adminCommunityLibrarySubmissionResolve(id), params) | ||
| .then(response => { | ||
| return response.data; | ||
| }); | ||
| }, |
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 usually map these resources to our Viewsets in Django, so it may be a better idea to have a AdminCommunityLibrarySubmissionResource here too.
Then on the submissions loading composable we can just do something like:
const resource = admin ? AdminCommunityLibrarySubmissionResource : CommunityLibrarySubmission;
resource.fetchCollection()...;| "latest_community_library_submission__id", | ||
| "latest_community_library_submission__status", | ||
| "has_any_live_community_library_submission", | ||
| "published_data", |
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.
Now that we will be loading the ChannelVersion model instances instead, we can get rid of this!
Summary
This PR implements a "Review submission" side panel in the administration dashboard for administrators to either approve submissions to the Community Library or flag them for review.
review_submission_side_panel_demo.webm
I have manually tested the changes by creating a Community Library submission for the channel, reviewing the submission in the Review submission panel, and then verifying that the correct data appears when the panel is reopened. I repeated these steps both for approving and flagging a submission. Finally, I have checked that if the "Cancel" button is clicked while submitting the review, it is not submitted.
Detailed Changes
ReviewSubmissionSidePanelcomponentChannelItemcomponentStatusChipcomponent toCommunityLibraryStatusChipand moved it under the "shared" directoryCommunityLibraryStatusChipstyling so that it works correctly in the new contextCommunityLibraryResolutionReasonconstants objectAdminChannelViewSetto includepublished_data(this has already been included for the user viewset, just not for the admin one)fetchCollectionAsAdminandresolveAsAdminmethods to theCommunityLibrarySubmissionresourceuseFetchanduseLatestCommunityLibrarySubmissioncomposables under the "shared" directoryuseFetchuseLatestCommunityLibrarySubmissionto include an admin argument, which determines whether the user endpoint or the admin endpoint should be usedusePublishedDatacomposable touseVersionDetail; this fixes a discrepancy from fdd5a84 where the implementation was changed to use version detail, but the naming was kept the same. This is not directly related to this PR, but I am including this change because it caused me some confusion in understanding that the handling of published data has been changed.References
Resolves #5301
Reviewer guidance
The changes should be fully testable withing the UI: create a channel, publish it, submit it to the community library, and then interact with this submission via the Review submission side panel.
Notes / Unresolved Things
UI/UX
Technical Details