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

feat: export tagged library as csv (TEMP) #628

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f84abd0
feat: export tagged library as csv
rpenido Feb 6, 2024
06810a2
Merge branch 'rpenido/fal-3610-download-course-tag-spreadsheet' into …
rpenido Feb 9, 2024
f750076
Merge branch 'rpenido/fal-3610-download-course-tag-spreadsheet' into …
rpenido Feb 10, 2024
f637076
Merge branch 'rpenido/fal-3610-download-course-tag-spreadsheet' into …
rpenido Feb 15, 2024
3cf462f
fix: rebasing
rpenido Feb 15, 2024
aa93510
fix: rename variable to fix typing
rpenido Feb 15, 2024
e94131f
fix: pylint
rpenido Feb 15, 2024
9a1729d
Merge branch 'rpenido/fal-3610-download-course-tag-spreadsheet' into …
rpenido Feb 15, 2024
6518a91
refactor: changing condition checking
rpenido Feb 15, 2024
8ddf875
fix: check invalidkey
rpenido Feb 15, 2024
663feed
feat: Upgrade Python dependency ora2 (#34244)
github-actions[bot] Feb 15, 2024
95b3e88
temp: add supplemental logging to debug IDV issues (#34248)
MichaelRoytman Feb 16, 2024
ed547be
fix: better error handling
rpenido Feb 16, 2024
df06d9a
docs: reverting unintended change
rpenido Feb 16, 2024
a153ec9
refactor: change functions to private
rpenido Feb 16, 2024
610c01a
refactor: cleaning build_object_tree_with_objecttags function
rpenido Feb 16, 2024
4012481
fix: explicit typing
rpenido Feb 16, 2024
4d1d82d
feat: export all course tags as csv (#34091)
rpenido Feb 16, 2024
6353bb2
feat: make FA form error messaging more descript (#34247)
christopappas Feb 16, 2024
4ad87c2
Merge branch 'master' into rpenido/fal-3611-download-library-tag-spre…
rpenido Feb 16, 2024
5caf952
fix: cleaning some types
rpenido Feb 16, 2024
df13345
docs: fix comment
rpenido Feb 16, 2024
b6366b6
perf: reduce number of queries for content tagging endpoints (#34200)
pomegranited Feb 16, 2024
e6a0dce
Merge branch 'master' into rpenido/fal-3611-download-library-tag-spre…
rpenido Feb 16, 2024
aa38f08
fix: typo
rpenido Feb 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions lms/djangoapps/verify_student/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,18 @@ def results_callback(request): # lint-amnesty, pylint: disable=too-many-stateme
reason = body_dict.get("Reason", "")
error_code = body_dict.get("MessageType", "")

# TODO: These logs must be removed once the investigation in COSMO-184 is complete.
# COSMO-196 was created to track the cleanup of these logs.
log.info(
"[COSMO-184] Software Secure review received for receipt_id={receipt_id}, "
"with result={result} and reason={reason}."
.format(
receipt_id=receipt_id,
result=result,
reason=reason,
)
)

try:
attempt = SoftwareSecurePhotoVerification.objects.get(receipt_id=receipt_id)
except SoftwareSecurePhotoVerification.DoesNotExist:
Expand Down Expand Up @@ -1099,6 +1111,10 @@ def results_callback(request): # lint-amnesty, pylint: disable=too-many-stateme
SoftwareSecurePhotoVerification.objects.filter(pk=previous_verification.pk
).update(expiry_email_date=None)
log.debug(f'Approving verification for {receipt_id}')

# TODO: These logs must be removed once the investigation in COSMO-184 is complete.
# COSMO-196 was created to track the cleanup of these logs.
log.info("[COSMO-184] Approved verification for receipt_id={receipt_id}.".format(receipt_id=receipt_id))
attempt.approve()

expiration_datetime = attempt.expiration_datetime.date()
Expand All @@ -1121,6 +1137,11 @@ def results_callback(request): # lint-amnesty, pylint: disable=too-many-stateme

elif result == "FAIL":
log.debug("Denying verification for %s", receipt_id)

# TODO: These logs must be removed once the investigation in COSMO-184 is complete.
# COSMO-196 was created to track the cleanup of these logs.
log.info("[COSMO-184] Denied verification for receipt_id={receipt_id}.".format(receipt_id=receipt_id))

attempt.deny(json.dumps(reason), error_code=error_code)
reverify_url = f'{settings.ACCOUNT_MICROFRONTEND_URL}/id-verification'
verification_status_email_vars['reasons'] = reason
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
return FormView.extend({
el: '.financial-assistance-wrapper',
events: {
'click .js-submit-form': 'submitForm'
'click .js-submit-form': 'submitForm',
'ajaxError': 'handleAjaxError'
},
tpl: formViewTpl,
fieldTpl: formFieldTpl,
Expand Down Expand Up @@ -101,6 +102,12 @@

if (error.status === 0) {
msg = gettext('An error has occurred. Check your Internet connection and try again.');
} else if (error.status === 403) {
txt = [
'You must confirm your email to complete registration before applying for financial assistance.',
'If you continue to have issues please contact support.'
],
msg = gettext(txt.join(' '));
}

this.errors = [HtmlUtils.joinHtml(
Expand Down Expand Up @@ -155,6 +162,12 @@
});
}
}
},

// this.model.save() makes an ajax call, which, when it errors,
// should have an error message displayed on the banner on the page
handleAjaxError: function (event, request, settings, thrownError) {
this.saveError(request);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ define([
expect(view.$('.js-success-message').length).toEqual(1);
};

failedSubmission = function() {
failedSubmission = function(statusCode) {
expect(view.$('.js-success-message').length).toEqual(0);
expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(0);
validSubmission();
view.model.trigger('error', {status: 500});
view.model.trigger('error', {status: statusCode});
expect(view.$('.js-success-message').length).toEqual(0);
expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(1);
};
Expand Down Expand Up @@ -166,7 +166,12 @@ define([
});

it('should submit the form and show an error message if content is valid and API returns error', function() {
failedSubmission();
failedSubmission(500);
});

it('should submit the form and show an error message if content is valid and API returns 403 error', function() {
failedSubmission(403);
expect(view.$('.message-copy').text()).toContain('You must confirm your email');
});

it('should allow form resubmission after a front end validation failure', function() {
Expand All @@ -176,7 +181,7 @@ define([
});

it('should allow form resubmission after an API error is returned', function() {
failedSubmission();
failedSubmission(500);
successfulSubmission();
});

Expand Down
21 changes: 11 additions & 10 deletions openedx/core/djangoapps/content_tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

import openedx_tagging.core.tagging.api as oel_tagging
from django.db.models import Exists, OuterRef, Q, QuerySet
from opaque_keys.edx.keys import CourseKey, LearningContextKey
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy
from organizations.models import Organization

Expand Down Expand Up @@ -87,7 +88,7 @@ def set_taxonomy_orgs(

def get_taxonomies_for_org(
enabled=True,
org_owner: Organization | None = None,
org_short_name: str | None = None,
) -> QuerySet:
"""
Generates a list of the enabled Taxonomies available for the given org, sorted by name.
Expand All @@ -100,7 +101,6 @@ def get_taxonomies_for_org(
If you want the disabled Taxonomies, pass enabled=False.
If you want all Taxonomies (both enabled and disabled), pass enabled=None.
"""
org_short_name = org_owner.short_name if org_owner else None
return oel_tagging.get_taxonomies(enabled=enabled).filter(
Exists(
TaxonomyOrg.get_relationships(
Expand Down Expand Up @@ -131,24 +131,25 @@ def get_unassigned_taxonomies(enabled=True) -> QuerySet:


def get_all_object_tags(
content_key: LearningContextKey,
content_key: LibraryLocatorV2 | CourseKey,
) -> tuple[ObjectTagByObjectIdDict, TaxonomyDict]:
"""
Returns a tuple with a dictionary of grouped object tags for all blocks and a dictionary of taxonomies.
"""
# ToDo: Add support for other content types (like LibraryContent and LibraryBlock)
context_key_str = str(content_key)
# We use a block_id_prefix (i.e. the modified course id) to get the tags for the children of the Content
# (course/library) in a single db query.
if isinstance(content_key, CourseKey):
course_key_str = str(content_key)
# We use a block_id_prefix (i.e. the modified course id) to get the tags for the children of the Content
# (course) in a single db query.
block_id_prefix = course_key_str.replace("course-v1:", "block-v1:", 1)
block_id_prefix = context_key_str.replace("course-v1:", "block-v1:", 1)
elif isinstance(content_key, LibraryLocatorV2):
block_id_prefix = context_key_str.replace("lib:", "lb:", 1)
else:
raise NotImplementedError(f"Invalid content_key: {type(content_key)} -> {content_key}")

# There is no API method in oel_tagging.api that does this yet,
# so for now we have to build the ORM query directly.
all_object_tags = list(ObjectTag.objects.filter(
Q(object_id__startswith=block_id_prefix) | Q(object_id=course_key_str),
Q(object_id__startswith=block_id_prefix) | Q(object_id=content_key),
Q(tag__isnull=False, tag__taxonomy__isnull=False),
).select_related("tag__taxonomy"))

Expand Down
27 changes: 16 additions & 11 deletions openedx/core/djangoapps/content_tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,21 @@ def get_relationships(

@classmethod
def get_organizations(
cls, taxonomy: Taxonomy, rel_type: RelType
) -> list[Organization]:
cls, taxonomy: Taxonomy, rel_type=RelType.OWNER,
) -> tuple[bool, list[Organization]]:
"""
Returns the list of Organizations which have the given relationship to the taxonomy.
Returns a tuple containing:
* bool: flag indicating whether "all organizations" have the given relationship to the taxonomy
* orgs: list of Organizations which have the given relationship to the taxonomy
"""
rels = cls.objects.filter(
taxonomy=taxonomy,
rel_type=rel_type,
)
# A relationship with org=None means all Organizations
if rels.filter(org=None).exists():
return list(Organization.objects.all())
return [rel.org for rel in rels]
is_all_org = False
orgs = []
# Iterate over the taxonomyorgs instead of filtering to take advantage of prefetched data.
for taxonomy_org in taxonomy.taxonomyorg_set.all():
if taxonomy_org.rel_type == rel_type:
if taxonomy_org.org is None:
is_all_org = True
else:
orgs.append(taxonomy_org.org)

return (is_all_org, orgs)
15 changes: 6 additions & 9 deletions openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from rest_framework.filters import BaseFilterBackend

import openedx_tagging.core.tagging.rules as oel_tagging
from organizations.models import Organization

from ...rules import get_admin_orgs, get_user_orgs
from ...models import TaxonomyOrg
Expand All @@ -25,9 +24,8 @@ def filter_queryset(self, request, queryset, _):
if oel_tagging.is_taxonomy_admin(request.user):
return queryset

orgs = list(Organization.objects.all())
user_admin_orgs = get_admin_orgs(request.user, orgs)
user_orgs = get_user_orgs(request.user, orgs) # Orgs that the user is a content creator or instructor
user_admin_orgs = get_admin_orgs(request.user)
user_orgs = get_user_orgs(request.user) # Orgs that the user is a content creator or instructor

if len(user_orgs) == 0 and len(user_admin_orgs) == 0:
return queryset.none()
Expand Down Expand Up @@ -67,11 +65,10 @@ class ObjectTagTaxonomyOrgFilterBackend(BaseFilterBackend):

def filter_queryset(self, request, queryset, _):
if oel_tagging.is_taxonomy_admin(request.user):
return queryset
return queryset.prefetch_related('taxonomy__taxonomyorg_set')

orgs = list(Organization.objects.all())
user_admin_orgs = get_admin_orgs(request.user, orgs)
user_orgs = get_user_orgs(request.user, orgs)
user_admin_orgs = get_admin_orgs(request.user)
user_orgs = get_user_orgs(request.user)
user_or_admin_orgs = list(set(user_orgs) | set(user_admin_orgs))

return queryset.filter(taxonomy__enabled=True).filter(
Expand All @@ -90,4 +87,4 @@ def filter_queryset(self, request, queryset, _):
)
)
)
)
).prefetch_related('taxonomy__taxonomyorg_set')
Loading
Loading