-
Notifications
You must be signed in to change notification settings - Fork 3
Endpoint for user program certificate info and program letter links #608
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
Endpoint for user program certificate info and program letter links #608
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #608 +/- ##
==========================================
+ Coverage 74.73% 74.81% +0.07%
==========================================
Files 245 245
Lines 11170 11204 +34
Branches 1943 1947 +4
==========================================
+ Hits 8348 8382 +34
Misses 2648 2648
Partials 174 174 ☔ View full report in Codecov by Sentry. |
mbertrand
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.
Looks good, but have some questions about the returned url and format.
| return reverse( | ||
| "profile:program-letter-intercept", args=[instance.micromasters_program_id] | ||
| ) | ||
|
|
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.
This is returning urls in the format "/program_letter/1/", which will only work for the certificate's logged-in user. Wondering if maybe it should provide the shareable url instead if it exists yet (/program_letter/<uuid>/view), or both as separate fields (something like create_letter_url, share_letter_url) since it's possible the shareable url hasn't yet been created?
Also, what do you think of returning absolute urls instead (http://<domain>/program_letter/....)? The frontend could create a full url itself, but maybe easier to do it in the backend?
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.
@mbertrand - made it generate absolute urls and changed the url params to return both a program_letter_generate_url and a program_letter_share_url. I also made it so the serializer itself does a get_or_create for the program letter. In the future we might consider removing the intercept view altogether but I think it still has a place for now (i'm imagning a case where we send links to view view a program letter from some marketing automation software that is only aware of program ids etc)
mbertrand
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.
👍
What are the relevant tickets?
Closes #556
Description (What does it do?)
This Pr adds an api endpoint (/api/v0/program_certificates) to retrieve program certificates along with program letter urls for an authenticated user. The idea is that when a user goes to a program specific dashboard, we will call this endpoint to get a list of program letter urls to display.
How can this be tested?