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

Dashboard: Adding "You passed this course" message with no certificates #5310

Merged
merged 5 commits into from
Mar 8, 2023

Conversation

annagav
Copy link
Contributor

@annagav annagav commented Feb 17, 2023

Pre-Flight checklist

  • Screenshots and design review for any changes that affect layout or styling
    • Desktop screenshots
    • Mobile width screenshots

What are the relevant tickets?

N/a

What's this PR do?

Update course status messages to show the message "You passed the course." for recent DEDP courses that that don't have a certificate.

How should this be manually tested?

Create a passing FinalGrade for a course run that has a start date after September 1st, 2022.
In the learner dashboard it should show a message the user passed the course.

Screen Shot 2023-02-22 at 8 13 03 AM

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2023

Codecov Report

Merging #5310 (70c770c) into master (9ffef4e) will increase coverage by 4.11%.
The diff coverage is 100.00%.

❗ Current head 70c770c differs from pull request most recent head 6b6334a. Consider uploading reports for the commit 6b6334a to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #5310      +/-   ##
==========================================
+ Coverage   93.61%   97.72%   +4.11%     
==========================================
  Files         499      299     -200     
  Lines       22975    13979    -8996     
  Branches      966        0     -966     
==========================================
- Hits        21508    13661    -7847     
+ Misses       1360      318    -1042     
+ Partials      107        0     -107     
Impacted Files Coverage Δ
...tic/js/components/CouponNotificationDialog_test.js 100.00% <ø> (ø)
static/js/factories/dashboard.js 100.00% <ø> (ø)
.../js/components/dashboard/courses/StatusMessages.js 93.47% <100.00%> (-0.14%) ⬇️
...omponents/dashboard/courses/StatusMessages_test.js 100.00% <100.00%> (ø)
dashboard/api.py
dashboard/utils.py
exams/api.py
exams/management/commands/list_paid_learners.py
dashboard/factories.py
discussions/urls.py
... and 194 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@arslanashraf7 arslanashraf7 self-assigned this Mar 7, 2023
Copy link
Contributor

@arslanashraf7 arslanashraf7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the code looks good but I added a comment where I think there might be a discrepancy in Course passed message and Passed badge between older courses.

Comment on lines +202 to +205
if (!course.certificate_url && course.is_passed) {
messages.push({
message: "You passed this course."
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there might be a problem with this If I'm assuming correctly.

I had a passed course (Analog Learning 300) in a program for which i also didn't have a certificate and i saw Course passed message along with a Passed badge this is how the dashboard looked:
image

After shifting to this PR, and adding FinalGrade record for another course i see the dashboard like below, where I don't see Course Passed message for Analog Learning 300 but i see Passed badge where as i see Course Passed message for Analog Learning 100 but no Passed badge. I think we need to make some tweaks in https://github.com/mitodl/micromasters/blob/master/static/js/lib/grades.js#L55 as well?

image

Copy link
Contributor Author

@annagav annagav Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try reproducing your scenario, but I did see the message. So I think what is happening is that the Course Run for which you have a passing grade doen't have the status set: FinalGrade.objects.filter(user=self.user, status=FinalGradeStatus.COMPLETE)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I might have messed up my data locally, if you are seeing the message and thePassed badge with this scenario it's fine then.

Copy link
Contributor

@arslanashraf7 arslanashraf7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 As per #5310 (comment)

@annagav annagav merged commit d40fa6d into master Mar 8, 2023
@odlbot odlbot mentioned this pull request Mar 9, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants