openedx / edx-platform

The Open edX LMS & Studio, powering education sites around the world!
https://openedx.org
GNU Affero General Public License v3.0
7.36k stars 3.85k forks source link

feat: added automatic hiding functionality for the survey report banner when clicking on the dismiss button #34914

Closed Asespinel closed 3 months ago

Asespinel commented 4 months ago

Description

This PR adds a basic functionality to automatically hide the survey report banner when the dismiss button is clicked for one month depending on the user that clicked on the button. This could be changed in the future if requested. We use the localStorage to check when the Dismiss button was clicked so we can hide the banner from the admin site.

Testing instructions

  1. Create 2 admin users and check the admin site separetly.
  2. With the first admin click on the Dismiss button and if you refresh the page, you should see how the banner dissapears after a short delay
  3. The second admin should see the banner appear normally
  4. Afterwards, if you click on the button with the second admin, the banner should hide automatically as well
  5. Is important to note that this dismiss variable is located in the local storage and you can clear it in your browser if necesarry if you want to do further testing
openedx-webhooks commented 4 months ago

Thanks for the pull request, @Asespinel! 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.

Asespinel commented 3 months ago

@mariajgrimaldi When you have the time could you review this PR?

Alec4r commented 3 months ago

Hi @Asespinel,

I have check the code and I think it's okay, but I think we could re factorize it and make it clean with some auxiliary functions.

Remind that if you need to set a lot of "code comments" could be a signal of your code it is not clear enough.

We could use some auxiliary functions to get the user id, the current time, set the localstorage, and validate if we should show or hide the banner.

Asespinel commented 3 months ago

Thanks for the suggested changes @Alec4r. I think this PR is ready to be reviewed. Can you check it when you have the chance @felipemontoya @mariajgrimaldi @ormsbee ?

mariajgrimaldi commented 3 months ago

I was going to review this but you folks have it covered. Thanks!

openedx-webhooks commented 3 months ago

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

edx-pipeline-bot commented 3 months ago

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot commented 3 months ago

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot commented 3 months ago

2U Release Notice: This PR has been deployed to the edX production environment.