mitodl / mitxonline

BSD 3-Clause "New" or "Revised" License
4 stars 2 forks source link

open edX: customize message after proctored exams are reviewed #374

Closed pdpinch closed 2 years ago

pdpinch commented 2 years ago

As the course team, I'd like the learner to see a different message on their exam page when the exam is reviewed so that they aren't confused.

Now, when learners complete a proctored exam and Verificient reviews it successfully they see a message that they should go to their progress page to see their exam.

Current message:

message

The progress page is hidden during the exam period so it is confusing to them. Can we change the messaging so it says "Exams are being reviewed and a final grade will be published soon" - or something to that effect?

It seems like the message to be changed is in this template, in the proctoring library:

https://github.com/openedx/edx-proctoring/blob/c3133b61052811e92f6cf0032a413db23cc909aa/edx_proctoring/templates/proctored_exam/verified.html#L6

Since this is a template, is it possible to override it in the theme?

If that's not an option, it would be best to add a feature to make this text customizable and then upstream it, so we don't have to maintain a fork.

Acceptance Criteria:

Out of Scope

pdpinch commented 2 years ago

I'm putting this on hold until we get some more details from the MITx EdTech team about how to reproduce this condition.

pdpinch commented 2 years ago

The steps to reproduce require integration with proctortrack, which isn't very convenient for developing and testing. I added a step to the acceptance criteria to research if there is a way to test that doesn't depend on a third party integration:

Steps to reproduce:

  1. Create a proctored exam(you may need to create an onboarding exam too, or you can set it so that an Onboarding exam is not required - you do this in the PT settings in Studio)
  2. Take proctored exam with a learner account making sure to pass all security measures - face scan, etc. (you can set it so there are fewer scans to do - also in PT settings in Studio)
  3. Submit proctored exam and wait for Verificient to review and verify it.
  4. Go back to proctored exam page and you should see that message there. I believe it is related to Line 1197 in https://github.com/openedx/edx-proctoring/blob/211ca866ae79e8f99ebab091a7a1abbd4d4ff791/edx_proctoring/locale/ro/LC_MESSAGES/djangojs.po
arslanashraf7 commented 2 years ago

Update:

Determine if there is a way to test this message without integrating with ProctorTrack. (It may help to post in the open edX forums. I've also found the edX proctoring team to be responsive on open edX slack in the #proctoring channel.)

There are a few things I found out while looking at this ticket for bypassing the proctoring setup/verification things and I could find below: 1) While the remote proctoring/proctor track sandbox configuration could be out of hand for devs, I could see this developer guidr that points out some steps to get proctor track simulation working with MockProck - I tried this and almost got the server running but i was always getting 404s. (This might be some config related thing i might be missing but the next solution works) 2) The other way around to bypass the proctoring requirements is as follows:

Determine if a template override is possible, using the mitxonline-theme

While looking at this point, I tried to override the Verified Template within the MITxOnline theme but couldn't get it to work. Further debugging in this part revealed that the messages we want to change are not coming from the edx-proctoring library) but they are coming from frontend-lib-special-exam instead, which is another MFE to support special exams.

Further, I'll look into the possibilities of overriding this within the MFE.

FYI, @pdpinch

briangrossman commented 2 years ago

@arslanashraf7 I spoke to a couple folks on the team and they would like the message to read:

Exams are being reviewed and a final grade will be available soon.

arslanashraf7 commented 2 years ago

Cross-posting some details:

While looking at the possibilities of overriding the text within the MFE's JSX files for changing this text there were a couple of documentations that I explored but couldn't identify if this could be changed without forking the frontend-lib-special-exams.

The documentation that was explored are: 1) Open edx branding and theming improvements 2) Theming and branding capabilities 3) Frontend Build

Probable Solutions: 1) The probable solutions included overriding the translation itself for the en language code, but for that we might have to serve the transifex with our own server instead of Edx's. 2) Fork the frontend-lib-special-exams, make our changes and override this library in our `frontend-app-learning fork](https://github.com/mitodl/frontend-app-learning?organization=mitodl&organization=mitodl)

It was decided to start as forking following solution#2 as mentioned above and for that, we'll fork the lib and override it. Along with starting a discussion in edx discussions and also try out & experiment making the texts configurable in our own fork of frontend-lib-special-exams

The discussion started here.

@pdpinch, What do you suggest for the name of the fork. Should it be something like frontend-lib-special-exams-mitol which would be something like we did for our frontend-component-header-mitol fork?

arslanashraf7 commented 2 years ago

@pdpinch, What do you suggest for the name of the fork. Should it be something like frontend-lib-special-exams-mitol which would be something like we did for our frontend-component-header-mitol fork?

@blarghmatey Could you share your thoughts on above?

blarghmatey commented 2 years ago

I think that name makes sense. I don't have any reason to diverge from that pattern.

pdpinch commented 2 years ago

@arslanashraf7 I think we should also try to get this text change merged upstream. It will probably take a long time because it will need edX/2U product input. So, we should keep our fork at frontend-component-header-mitol as well. We would like to deploy this change in time for the April exams.

arslanashraf7 commented 2 years ago

@pdpinch, FYI

So, we should keep our fork at frontend-component-header-mitol as well

The fork is at https://github.com/mitodl/frontend-lib-special-exams-mitol

@arslanashraf7 I think we should also try to get this text change merged upstream. It will probably take a long time because it will need edX/2U product input

The upstream PR is at https://github.com/edx/frontend-lib-special-exams/pull/63

arslanashraf7 commented 2 years ago

So, After the upstream PR(https://github.com/edx/frontend-lib-special-exams/pull/63) with the required change was merged.

The change is now part of Release 1.16.1 as you can see here.

So, we need to get to the 1.16.1 version of the frontend-lib-special-exam. While looking at the possibilities of getting this change to the MITx instances of edX and these are some possible options for us as of now that depend upon the following:

1) Do we want to get to the latest edX's master before the exam?

This is feasible since the change is now part of exam-lib's master. The exam-lib version used by frontend-app-learning is 1.16.0. So, first, we need to bump exam-lib version in frontend-app-leaning, I was going to create a PR for that but then I noticed a Renovate automated PR that already does that but it's not merged yet so we'll need to ask someone to get it merged.

If we go with option#1 we just need to get this Renovate PR merged are then deploy the latest edx master as we usually do.

2) We only want this change without pulling the latest master?

In this case, if we don't feel confident enough to update to the latest edX master as mentioned in option#1, we can try and override the latest package from special-exam in frontend-app-learning in ol-infractructure repo. Please note that this change will only be valid until we get to the Nutmeg release of edX or the latest master as I mentioned above.

FYI, @briangrossman , @pdpinch

arslanashraf7 commented 2 years ago

Should we close this now @briangrossman ?

arslanashraf7 commented 2 years ago

Closing this issue via https://github.com/edx/frontend-lib-special-exams/pull/63. The PR was merged upstream and the change should be available in the Nutmeg & master.