openedx / frontend-app-gradebook

Instructor grade book tool
GNU Affero General Public License v3.0
11 stars 87 forks source link

fix: replace Field.Email reference for Field.Text #345

Closed dcoa closed 1 year ago

dcoa commented 1 year ago

Description The master branch doesn't have the references to new Field.Text updated, there is a screenshot of the error

image

As well useGradeButtonData doesn't return a string in the label field is returning a function, so the numbers weren't render image

This PR tries to solve that rendering situation.

What changed?

Developer Checklist

Reviewer Checklist

Collectively, these should be completed by reviewers of this PR:

FYI: @openedx/content-aurora

openedx-webhooks commented 1 year ago

Thanks for the pull request, @dcoa! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (44197f6) 94.93% compared to head (e10ef6d) 94.93%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #345 +/- ## ======================================= Coverage 94.93% 94.93% ======================================= Files 139 139 Lines 1343 1343 Branches 264 264 ======================================= Hits 1275 1275 Misses 60 60 Partials 8 8 ``` | [Files Changed](https://app.codecov.io/gh/openedx/frontend-app-gradebook/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx) | Coverage Δ | | |---|---|---| | [src/components/GradesView/GradebookTable/hooks.jsx](https://app.codecov.io/gh/openedx/frontend-app-gradebook/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx#diff-c3JjL2NvbXBvbmVudHMvR3JhZGVzVmlldy9HcmFkZWJvb2tUYWJsZS9ob29rcy5qc3g=) | `100.00% <ø> (ø)` | | | [...mponents/GradesView/GradebookTable/GradeButton.jsx](https://app.codecov.io/gh/openedx/frontend-app-gradebook/pull/345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx#diff-c3JjL2NvbXBvbmVudHMvR3JhZGVzVmlldy9HcmFkZWJvb2tUYWJsZS9HcmFkZUJ1dHRvbi5qc3g=) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arslanashraf7 commented 1 year ago

Hey @mphilbrick211 we are facing the same issue while running the gradebook locally and on a server. Is it possible by any chance that the review on this PR could be prioritized?

mphilbrick211 commented 1 year ago

Hey @mphilbrick211 we are facing the same issue while running the gradebook locally and on a server. Is it possible by any chance that the review on this PR could be prioritized?

Hi @openedx/content-aurora! Flagging this - would someone be able to please review?

dcoa commented 1 year ago

@muselesscreator @leangseu-edx

Hi folks, could you help us with a review in his PR or help us identify the right reviewer?

dcoa commented 1 year ago

Hi @arbrandes thank you for your review and not 100% sure to understand your request because when I used the branch I realized that the reason why the test didn't catch the errors was because the testing for this component and function was correct: Field.Text test and subsectionGrade test only the reference was not updated and not fail because of the use of jest.mock.

I updated the test reference too, or I just can think of using the original component instead of mocking it but that does not make the test fail only displays a console.error

arbrandes commented 1 year ago

Yeah, it might be going overboard to test for this retroactively, here. I feel like it would initiate a full revamp, which would probably be better planned elsewhere. (As part of the migration to react-testing-library, perhaps.) Let's just fix the glaring issue and move on, for now.

openedx-webhooks commented 1 year ago

@dcoa 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.