lifterlms
lifterlms copied to clipboard
{{student_name}} merge code for achievements says "you" instead of student name when printed
Reproduction Steps
This may not necessarily be a bug so much as a point of confusion.
- Trigger any of the basic notifications that have the {{student_name}} merge code
- go into the dashboard and look at the notification
Expected Behavior
- It should read student name but instead says you:
Actual Behavior
- This makes sense with our default layout! However, when users customize this there is an expectation that student_name should be the student's name. So it makes less sense when they say Congratulations student_name, you successfully enrolled in x. It reads on the front end as Congratulations You, you successfully enrolled in x.
Error Messages / Logs
- Include any relevant error messages or log files
<!-- Paste error logs / backtraces below this line -->
System and Environment Information
System Report
This issue has be recreated:
- [X] Locally
- [ ] On a staging site
- [X] On a production website
- [X] With only LifterLMS and a default theme
Browser, Device, and Operating System Information
- Browser name and version
- Operating System name and version
- Device name and version (if applicable)
HS-158340
@nrherron92
For a long time I've been trying to argue that this is not a bug but is, instead, expected behavior. I always argue "Why would you want to say someone's name to them on the screen. It's obviously a notification for you" .
If I'm looking at any notification's on my phone, let's say Rocco DMs me on instagram it says "Rocco has sent you a new message" not "Rocco has sent Thomas Levy" a new message.
That said... the merge code says "Student Name" and if the user wants to output the name (which I think is silly) instead of "You" when a course is completed or whatever they should be able to.
I think we should just change the behavior instead of me trying to argue a (dumb) point.
@eri-trabiccolo thoughts on this?
@thomasplevy @nrherron92 I think I agree with Natalie.
HS-173338
@nrherron92 prioritized