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

Add Reset or Remove Progress action item #5031

Merged

Conversation

gabrielcaires
Copy link
Contributor

Related to #4959

Changes proposed in this Pull Request

  • it adds a Remove or Reset Progress item on the student action menu.

Testing instructions

  • Go to the students' page
  • Click on the 3 dots point
  • Check if the menu item Reset Or Remove progress is available
  • Click to open the modal
  • Check if the modal is related to reset the progress
  • Reset a student's progress, choosing a course and applying the reset.

Screenshot / Video

Screen Shot 2022-04-14 at 17 43 42

Screen Shot 2022-04-14 at 17 43 57

@LavaGolem
Copy link
Contributor

LavaGolem commented Apr 18, 2022

Works as expected 🗡️

Do we need package-lock.json pushed here?

@gabrielcaires
Copy link
Contributor Author

Works as expected 🗡️

Do we need package-lock.json pushed here?

Removed! Thanks @LavaGolem Can you approve the PR?

@merkushin
Copy link
Member

merkushin commented Apr 18, 2022

@gabrielcaires, does it contain your fix for DELETE method?
I got 405 Method Not Allowed when tried to reset progress.

Hmm, and for some reason the button in modal looks different for me (comparing to the screenshot in the description):
CleanShot 2022-04-18 at 19 19 57@2x

Maybe that's some cache on my side =/

@gabrielcaires
Copy link
Contributor Author

@merkushin No, the fix is here #5029.
I am going to merge 5029 and then I can rebase here.

@gabrielcaires gabrielcaires force-pushed the add/add-reset-or-remove-action-link branch from 03297d7 to e81fbbd Compare April 18, 2022 16:34
@gabrielcaires
Copy link
Contributor Author

gabrielcaires commented Apr 18, 2022

@merkushin Rebased.
I noticed a weird behavior.
I saw that the endpoint is resetting the student's progress but the Course Progress it still says that the student has progress.

Image after reset the progress
Screen Shot 2022-04-18 at 13 43 21

*Image on the course details *

Screen Shot 2022-04-18 at 14 13 21

I think it is wrong, but considering we are removing this column, I think we don't need a new card, right?

Hmm, and for some reason, the button in modal looks different for me (compared to the screenshot in the description):

Hum, are you using which npm version? maybe a new npm install + npm build ?
I already saw something similar.

@merkushin
Copy link
Member

@gabrielcaires

I saw that the endpoint is resetting the student's progress but the Course Progress it still says that the student has progress.

I think I will fix it in my current card (Replace "Course Progress" with "Enrolled Courses" column).

Hum, are you using which npm version? maybe a new npm install + npm build ?

That's my standard routine after changing the current branch :)

Copy link
Collaborator

@donnapep donnapep left a comment

Choose a reason for hiding this comment

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

I saw that the endpoint is resetting the student's progress but the Course Progress it still says that the student has progress.

If a student hasn't started a course (i.e. the Status column is Not Started on the course details page), then choosing Reset or Remove Progress will remove their progress (i.e. the row for that student will be removed).

If a student has started a course (i.e. the Status column is In Progress on the course details page), then choosing Reset or Remove Progress will reset their progress to Not Started.

So it's currently working as intended and there isn't actually anything to fix here. I tested and saw that when a student has not started a course yet, choosing Reset or Remove Progress successfully removes the course from the Course Progress column.

I also see the same thing as @merkushin wrt to the button and have been meaning to look into it. This could be addressed in a separate PR though, especially since you're not able to reproduce it (I don't get checkmarks showing in the checkbox either, which is also weird):

Screen Shot 2022-04-19 at 4 06 48 PM

@donnapep donnapep merged commit 7bd43ce into feature/student-management Apr 20, 2022
@donnapep donnapep deleted the add/add-reset-or-remove-action-link branch April 20, 2022 13:51
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.

4 participants