-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix: replace username with full name in the learner record title #294
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #294 +/- ##
==========================================
+ Coverage 70.22% 70.37% +0.14%
==========================================
Files 27 27
Lines 403 405 +2
Branches 85 87 +2
==========================================
+ Hits 283 285 +2
Misses 119 119
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Why is the username being removed? Has this gotten product review, yet? Apologies if it already has, but I'd like this to be confirmed before we merge it. |
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.
Pending product review question.
@arbrandes I was expecting this to be swapping username for full name, which made sense to us since this is a transcript page, basically. Email should be retained because it's used by the credit transfer feature. Open to leaving username if needed, but typically full name is more appropriate for a certificate record. |
@hurtstotouchfire, thanks for the description of the intended behavior. The link in the ticket description is private, unfortunately, so there was no way to understand the reasoning behind the change other than figuring it out from the code. Still, it's a user-facing change, and we should probably get somebody from Product giving it a 👍🏼. Adding the label in. |
Sounds good, thanks |
38863cd
to
2010a4f
Compare
2010a4f
to
8b082c8
Compare
It makes sense to me to use the full name instead of the username. I'm just wondering if there are any implications on the data pipeline for reporting purposes? Does this change completely remove the association of usernames from learner records? If so, does that impact any future plans for reports that may touch learner records? cc @bmtcril @crathbun428 |
This is just a user facing change. The username has been replaced with full name only on the frontend. All the existing functionality that depends on username for learner record has not been touched. |
@jmakowski1123 @bmtcril - Capturing this question Brian raised here since I think its an important one. Curious how an instructor may handle identifying a learner that has changed their email address if they have a number of learners with the same name. Will there be a way of identifying those learners in this case? |
Got it, thanks @zainab-amir . @crathbun428 Since this is just a frontend change, I'm assuming the data pipeline won't be affected since the user ids aren't being removed from the backend. Something to pass back to Brian, but also to keep in mind down the road if the conversation evolves to removing usernames entirely (which I think is a future state discovery conversation, but not being acted on at the moment). This has product aproval! |
Description: Remove username from learner record title VAN-1832
8b082c8
to
4fbe8c7
Compare
Description: Replace username with full name VAN-1832
4fbe8c7
to
22c4c9c
Compare
@crathbun428 This page is a learner-facing transcript. Instructors have no way of navigating to this page. This transcript is used by the learner to track their own progress and can also be shared publicly (like with a CV or sending to a university for transfer credit). |
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 to me!
Description: Remove username from learner record title and replace it with Full Name
VAN-1832