Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: Add highlight content card to stepper and individual sets #912
feat: Add highlight content card to stepper and individual sets #912
Changes from 2 commits
ee6e285
d1e2ec0
8c09af9
5de4ae0
f699452
cd6cdae
97876cc
db03859
58e5861
e7bafe7
1e9f486
edce596
d77cd5e
dc1d591
60587a7
7bc6c4f
cd1782c
722df03
17b8baa
d1d51b0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hmm, while this technically should work, it may not be optimal. That is, you're parsing the
content_key
from theaggregation_key
field for the course. While the latter part of theaggregation_key
is the format ofcontent_key
that enterprise-catalog expects here, it is a bit risky to assume that thecontent_key
can be derived from theaggregation_key
in this way, if that makes sense.For example,
content_key
for courses in enterprise-catalog is derived from thekey
field, whereascontent_key
for programs/pathways content in enterprise-catalog is derived from theuuid
field.I think what you have now should be fine for now because your assumption is currently true, but we may want to add a TODO comment here to suggest finding a solution of extracting the appropriate
content_key
value from each selected highlighted content item based on its content type (i.e.,key
for courses, anduuid
for programs/pathways).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 might try rubber ducking later on (outside of this PR) on finding a more optimal solution here.
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.
Leaving Open as reference in the future