feat: Use configured DEFAULT_GRADE_DESIGNATIONS #1227

xitij2000 · 2024-08-23T14:08:23Z

Description

Support was recently added to edx-platform to add customized default grade designations, this change adds support for them to this MFE as well to bring it to parity with the edx-platform UI.

It also refactors the grading-settings page to use React Query and updates the logic used when partitioning grades by default to make it work better when there are more than 5 partitions.

The original logic was to subdivide each segment as half of the previous segment, so when you go from Pass/Fail to A, B, F, it will keep F at 50% and split Pass into A and B of 25% each. Adding another segment will then split B into B and C each getting 12% roughly, adding another segment would now split C into C and D of roughly 6% each. Adding yet another segment at this point would then cause the splits of 3% and make the segment too small to be usable.

This change will update this to switch to making even splits when you go beyond 5 segments so that all grades get equal space. Author's can then choose to readjust as needed.

Testing instructions

Change DEFAULT_GRADE_DESIGNATIONS in cms's settings
Visit the grading setting for a course.
Add segments to the grading scale. It will stop after 5 divisions A, B, C, D, F, and the DEFAULT_GRADE_DESIGNATIONS will be ignored
Check out this PR.
The segment names should now be pulled from DEFAULT_GRADE_DESIGNATIONS.

Other information

Screencast_20240830_144834.webm

Screencast_20240830_144909.webm

openedx-webhooks · 2024-08-23T14:08:27Z

Thanks for the pull request, @xitij2000!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2024-08-26T10:36:58Z

Codecov Report

Attention: Patch coverage is 98.52941% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.24%. Comparing base (774728a) to head (53519db).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...rc/grading-settings/grading-scale/GradingScale.jsx	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1227      +/-   ##
==========================================
+ Coverage   93.16%   93.24%   +0.07%     
==========================================
  Files        1054     1052       -2     
  Lines       20467    20450      -17     
  Branches     4358     4289      -69     
==========================================
  Hits        19068    19068              
+ Misses       1339     1322      -17     
  Partials       60       60

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Cup0fCoffee

Requires fixing.

Cup0fCoffee · 2024-08-28T19:07:32Z

src/grading-settings/grading-scale/GradingScale.scss

@@ -81,7 +81,7 @@
      .grading-scale-segment-content {
        display: flex;
        flex-direction: column;
-        margin-right: 1.25rem;
+        //margin-right: 1.25rem;


Cup0fCoffee · 2024-08-28T19:17:35Z

src/grading-settings/grading-scale/components/GradingScaleSegment.tsx

+        <input
+          className="grading-scale-segment-content-title m-0"
+          data-testid="grading-scale-segment-input"
+          value={getLettersOnLongScale(idx, letters, gradingSegments)}


@xitij2000 I think the description of the getLettersOnShortScale and it's long alternatives are wrong.

Cup0fCoffee · 2024-08-28T20:06:39Z

src/grading-settings/GradingSettings.jsx

@@ -156,6 +160,7 @@ const GradingSettings = ({ intl, courseId }) => {
                      resetDataRef={resetDataRef}
                      setOverrideInternetConnectionAlert={setOverrideInternetConnectionAlert}
                      setEligibleGrade={setEligibleGrade}
+                      defaultGradeDesignations={gradingSettings.defaultGradeDesignations}


@xitij2000 When trying to test the changes locally I'm getting an error on this line, since gradingSettings doesn't seem to be defined.

Surprisingly I never faced this issue, but I can understand why it'd happen. Hopefully the latest commits fix this.

Cup0fCoffee · 2024-08-29T15:51:07Z

👍 Please update the PR description by adding the testing instructions. After that can be merged.

I tested this: tested that if DEFAULT_GRADE_DESIGNATIONS has a size greater than 5, you can add that many grade threadsholds in the MFE UI (before that 5 was the limit).
I read through the code
[-] I checked for accessibility issues
[-] Includes documentation
[-] I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

xitij2000 · 2024-08-30T09:28:28Z

src/grading-settings/grading-scale/GradingScale.scss

@@ -9,6 +9,7 @@
    height: 3.125rem;
    width: 100%;
    border: 1px solid $black;
+    overflow: hidden;


This is added because the text elements in the segment can overflow and interfere with the add segment button.

xitij2000 · 2024-08-30T09:29:09Z

src/grading-settings/grading-scale/GradingScale.jsx

+      if (prevSegments.length >= 5) {
+        const segSize = MAXIMUM_SCALE_LENGTH / (prevSegments.length + 1);
+        return Array.from({ length: prevSegments.length + 1 }).map((_, i) => (
+          {
+            current: 100 - i * segSize,
+            previous: 100 - (i + 1) * segSize,
+          }
+        ));
+      }


If the number of segments is more than 5 this will switch to just splitting the segments evenly.

because of the return statement here the "showSavePrompt" below is not triggered I am investigating a bit more.

@farhaanbukhsh Thanks for identifying an fixing this! I'll incorporate your fix into my pr.

rayzhou-bit

I tested this in hosted devstack and it looks good to me!

farhaanbukhsh · 2024-10-24T14:20:04Z

@xitij2000 Please have a look at open-craft#73 something that should be included in the PR, we can just cherry-pick the commit :)

Support was recently added to edx-platform to add customisised default grade designations, this change adds support for them to this MFE as well to bring it to partiy with the edx-platform UI It also refactors the grading-settings page to use React Query and updates the logic used when partitioning grades by default to make it work better when there are more than 5 partitions. Co-authored-by: Farhaan Bukhsh <[email protected]>

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 23, 2024

xitij2000 force-pushed the kshitij/use-custom-grade-range branch 3 times, most recently from f96bc89 to 519ee38 Compare August 26, 2024 10:27

xitij2000 marked this pull request as ready for review August 27, 2024 08:28

xitij2000 requested a review from a team as a code owner August 27, 2024 08:28

Cup0fCoffee suggested changes Aug 28, 2024

View reviewed changes

xitij2000 force-pushed the kshitij/use-custom-grade-range branch from e468f2b to 0c1d685 Compare August 30, 2024 09:26

xitij2000 commented Aug 30, 2024

View reviewed changes

Cup0fCoffee mentioned this pull request Aug 30, 2024

feat: Use configured DEFAULT_GRADE_DESIGNATIONS open-craft/frontend-app-authoring#62

Merged

rayzhou-bit approved these changes Sep 26, 2024

View reviewed changes

farhaanbukhsh mentioned this pull request Oct 24, 2024

Fixes the saving pop-up to appear open-craft/frontend-app-authoring#73

Merged

xitij2000 force-pushed the kshitij/use-custom-grade-range branch from 7930f89 to dabfc0b Compare October 25, 2024 08:18

xitij2000 force-pushed the kshitij/use-custom-grade-range branch from dabfc0b to 53519db Compare October 25, 2024 08:18

xitij2000 merged commit 4d4adce into master Oct 25, 2024
8 checks passed

xitij2000 deleted the kshitij/use-custom-grade-range branch October 25, 2024 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use configured DEFAULT_GRADE_DESIGNATIONS #1227

feat: Use configured DEFAULT_GRADE_DESIGNATIONS #1227

xitij2000 commented Aug 23, 2024 •

edited

Loading

openedx-webhooks commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

Cup0fCoffee left a comment

Cup0fCoffee Aug 28, 2024

Cup0fCoffee Aug 28, 2024

Cup0fCoffee Aug 28, 2024

xitij2000 Aug 29, 2024

Cup0fCoffee commented Aug 29, 2024

xitij2000 Aug 30, 2024

xitij2000 Aug 30, 2024

farhaanbukhsh Oct 21, 2024

xitij2000 Oct 25, 2024

rayzhou-bit left a comment

farhaanbukhsh commented Oct 24, 2024

feat: Use configured DEFAULT_GRADE_DESIGNATIONS #1227

feat: Use configured DEFAULT_GRADE_DESIGNATIONS #1227

Conversation

xitij2000 commented Aug 23, 2024 • edited Loading

Description

Testing instructions

Other information

openedx-webhooks commented Aug 23, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Aug 26, 2024 • edited Loading

Codecov Report

Cup0fCoffee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cup0fCoffee commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayzhou-bit left a comment

Choose a reason for hiding this comment

farhaanbukhsh commented Oct 24, 2024

xitij2000 commented Aug 23, 2024 •

edited

Loading

openedx-webhooks commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading