-
Notifications
You must be signed in to change notification settings - Fork 278
Compute license audit task results during publish workflow #5665
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that we are getting this data from the |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,128 +1,51 @@ | ||
| import { computed, ref, unref, watch } from 'vue'; | ||
| import { Channel } from 'shared/data/resources'; | ||
|
|
||
| export function useLicenseAudit(channelRef, channelVersionRef) { | ||
| const isAuditing = ref(false); | ||
| const auditTaskId = ref(null); | ||
| const auditError = ref(null); | ||
| const publishedData = ref(null); | ||
|
|
||
| watch( | ||
| () => unref(channelRef)?.published_data, | ||
| newPublishedData => { | ||
| if (newPublishedData) { | ||
| publishedData.value = newPublishedData; | ||
| if (isAuditing.value) { | ||
| isAuditing.value = false; | ||
| auditError.value = null; | ||
| } | ||
| } | ||
| }, | ||
| { immediate: true, deep: true }, | ||
| ); | ||
|
|
||
| const currentVersionData = computed(() => { | ||
| const version = unref(channelVersionRef); | ||
| if (!publishedData.value || version == null) { | ||
| return undefined; | ||
| } | ||
| return publishedData.value[version]; | ||
| }); | ||
| import { computed, unref } from 'vue'; | ||
|
|
||
| export function useLicenseAudit(versionDetailRef, isLoadingRef, isFinishedRef) { | ||
| const hasAuditData = computed(() => { | ||
| const versionData = currentVersionData.value; | ||
| const versionData = unref(versionDetailRef); | ||
| if (!versionData) { | ||
| return false; | ||
| } | ||
|
|
||
| return ( | ||
| 'non_distributable_licenses_included' in versionData && | ||
| 'special_permissions_included' in versionData | ||
| 'non_distributable_included_licenses' in versionData && | ||
| 'included_special_permissions' in versionData | ||
| ); | ||
| }); | ||
|
|
||
| const invalidLicenses = computed(() => { | ||
| const versionData = currentVersionData.value; | ||
| return versionData?.non_distributable_licenses_included || []; | ||
| const versionData = unref(versionDetailRef); | ||
| return versionData?.non_distributable_included_licenses || []; | ||
| }); | ||
|
|
||
| const specialPermissions = computed(() => { | ||
| const versionData = currentVersionData.value; | ||
| return versionData?.special_permissions_included || []; | ||
| const versionData = unref(versionDetailRef); | ||
| return versionData?.included_special_permissions || []; | ||
| }); | ||
|
|
||
| const includedLicenses = computed(() => { | ||
| const versionData = currentVersionData.value; | ||
| const versionData = unref(versionDetailRef); | ||
| return versionData?.included_licenses || []; | ||
| }); | ||
|
|
||
| const isAuditComplete = computed(() => { | ||
| return publishedData.value !== null && hasAuditData.value; | ||
| }); | ||
|
|
||
| async function triggerAudit() { | ||
| if (isAuditing.value) return; | ||
|
|
||
| try { | ||
| isAuditing.value = true; | ||
| auditError.value = null; | ||
|
|
||
| const channelId = unref(channelRef)?.id; | ||
| if (!channelId) { | ||
| throw new Error('Channel ID is required to trigger audit'); | ||
| } | ||
|
|
||
| const response = await Channel.auditLicenses(channelId); | ||
| auditTaskId.value = response.task_id; | ||
| } catch (error) { | ||
| isAuditing.value = false; | ||
| auditError.value = error; | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async function fetchPublishedData() { | ||
| const channelId = unref(channelRef)?.id; | ||
| if (!channelId) return; | ||
|
|
||
| try { | ||
| const data = await Channel.getPublishedData(channelId); | ||
| publishedData.value = data; | ||
| } catch (error) { | ||
| auditError.value = error; | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async function checkAndTriggerAudit() { | ||
| if (!publishedData.value) { | ||
| await fetchPublishedData(); | ||
| } | ||
|
|
||
| if (hasAuditData.value || isAuditing.value) { | ||
| return; | ||
| } | ||
|
|
||
| await triggerAudit(); | ||
| } | ||
|
|
||
| return { | ||
| isLoading: computed(() => { | ||
| if (isAuditComplete.value || auditError.value) return false; | ||
| return isAuditing.value; | ||
| const loading = unref(isLoadingRef); | ||
| if (typeof loading === 'boolean') { | ||
| return loading; | ||
| } | ||
| return !unref(versionDetailRef); | ||
| }), | ||
| isFinished: computed(() => { | ||
|
Comment on lines
+33
to
+39
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we won't need to care about |
||
| const finished = unref(isFinishedRef); | ||
| if (typeof finished === 'boolean') { | ||
| return finished; | ||
| } | ||
| return Boolean(unref(versionDetailRef)); | ||
| }), | ||
| isFinished: computed(() => isAuditComplete.value), | ||
| isAuditing, | ||
| invalidLicenses, | ||
| specialPermissions, | ||
| includedLicenses, | ||
| hasAuditData, | ||
| auditTaskId, | ||
| error: auditError, | ||
|
|
||
| checkAndTriggerAudit, | ||
| triggerAudit, | ||
| fetchPublishedData, | ||
| currentVersionData, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -439,8 +439,7 @@ | |
| isFinished: licenseAuditIsFinished, | ||
| invalidLicenses, | ||
| includedLicenses, | ||
| checkAndTriggerAudit: checkAndTriggerLicenseAudit, | ||
| } = useLicenseAudit(props.channel, currentChannelVersion); | ||
| } = useLicenseAudit(versionDetail, publishedDataIsLoading, publishedDataIsFinished); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove this call, and compute |
||
|
|
||
| const allSpecialPermissionsChecked = ref(true); | ||
|
|
||
|
|
@@ -470,7 +469,6 @@ | |
| watch(isPublishing, async (newIsPublishing, oldIsPublishing) => { | ||
| if (oldIsPublishing === true && newIsPublishing === false) { | ||
| await fetchPublishedData(); | ||
| await checkAndTriggerLicenseAudit(); | ||
| } | ||
| }); | ||
|
|
||
|
|
@@ -479,7 +477,6 @@ | |
|
|
||
| if (!isPublishing.value) { | ||
| await fetchPublishedData(); | ||
| await checkAndTriggerLicenseAudit(); | ||
| } | ||
| }); | ||
|
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove this command; we will handle this more broadly in #5593
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, after we remove this, we can remove the |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| from django.core.management.base import BaseCommand | ||
|
|
||
| from contentcuration.models import Channel | ||
| from contentcuration.models import ChannelVersion | ||
| from contentcuration.utils.audit_channel_licenses import audit_channel_version | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = "Backfill license audit results for published channels." | ||
|
|
||
| def add_arguments(self, parser): | ||
| parser.add_argument( | ||
| "--channel-id", | ||
| action="append", | ||
| dest="channel_ids", | ||
| help="Channel ID to backfill (repeatable).", | ||
| ) | ||
| parser.add_argument( | ||
| "--limit", | ||
| type=int, | ||
| default=None, | ||
| help="Limit number of channels to process.", | ||
| ) | ||
| parser.add_argument( | ||
| "--offset", | ||
| type=int, | ||
| default=0, | ||
| help="Offset into the channel list.", | ||
| ) | ||
| parser.add_argument( | ||
| "--dry-run", | ||
| action="store_true", | ||
| help="Report what would be updated without saving.", | ||
| ) | ||
|
|
||
| def handle(self, *args, **options): | ||
| channel_ids = options.get("channel_ids") | ||
| limit = options.get("limit") | ||
| offset = options.get("offset") or 0 | ||
| dry_run = options.get("dry_run") | ||
|
|
||
| queryset = Channel.objects.filter( | ||
| main_tree__published=True, | ||
| version__gt=0, | ||
| ).order_by("id") | ||
|
|
||
| if channel_ids: | ||
| queryset = queryset.filter(id__in=channel_ids) | ||
|
|
||
| if offset: | ||
| queryset = queryset[offset:] | ||
|
|
||
| if limit: | ||
| queryset = queryset[:limit] | ||
|
|
||
| processed = 0 | ||
| failures = 0 | ||
|
|
||
| for channel in queryset.iterator(): | ||
| processed += 1 | ||
| try: | ||
| channel_version, _ = ChannelVersion.objects.get_or_create( | ||
| channel=channel, | ||
| version=channel.version, | ||
| ) | ||
| if channel.version_info_id != channel_version.id: | ||
| if dry_run: | ||
| self.stdout.write( | ||
| f"Would set version_info for channel {channel.id}" | ||
| ) | ||
| else: | ||
| Channel.objects.filter(pk=channel.pk).update( | ||
| version_info=channel_version | ||
| ) | ||
|
|
||
| if dry_run: | ||
| self.stdout.write( | ||
| f"Would backfill audit results for channel {channel.id} " | ||
| f"version {channel_version.version}" | ||
| ) | ||
| continue | ||
|
|
||
| audit_channel_version(channel_version) | ||
| self.stdout.write( | ||
| f"Backfilled audit results for channel {channel.id} " | ||
| f"version {channel_version.version}" | ||
| ) | ||
| except Exception as error: # noqa: BLE001 | ||
| failures += 1 | ||
| self.stderr.write(f"Failed to backfill channel {channel.id}: {error}") | ||
|
|
||
| self.stdout.write( | ||
| f"Backfill complete. Processed={processed} Failures={failures}" | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from django.db import migrations | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("contentcuration", "0161_update_channelversion_choices"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.RenameField( | ||
| model_name="channelversion", | ||
| old_name="non_distributable_licenses_included", | ||
| new_name="non_distributable_included_licenses", | ||
| ), | ||
| migrations.RenameField( | ||
| model_name="channelversion", | ||
| old_name="special_permissions_included", | ||
| new_name="included_special_permissions", | ||
| ), | ||
| ] |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, apologies for the misdirection, we had already changed the name of these propoerties, so we can keep with the |
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.
Oh, we don't need to run this migration in the scope of this PR, we will handle it in #5593! 👐