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

Adds ability to clear onboarding for a worker #886

Merged
merged 4 commits into from
Aug 10, 2022

Conversation

Etesam913
Copy link
Contributor

Overview

For development purposes it can be convenient to reset a worker's onboarding qualifications. Now it is possible!

Changes:

  • Added clear_onboarding(self, worker: "Worker", qualification_name: str): to onboarding_required.py as a class method.
  • Added a clear_worker_onboarding.py script that can used to clear a worker's onboarding qualifications.
    • This script can be ran by typing mephisto scripts local_db clear_worker_onboarding in the console.

Video:

clear_onboarding.mp4

This video shows how the script can be used to clear a worker's onboarding. Worker x gets the onboarding qualification granted when he presses the button. After the script is ran, the worker is taken back to the onboarding page as the script revoked the onboarding qualification.

Resolves #790

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2022

Codecov Report

Merging #886 (3af2e40) into main (b165bb2) will increase coverage by 0.04%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main     #886      +/-   ##
==========================================
+ Coverage   64.65%   64.69%   +0.04%     
==========================================
  Files         108      108              
  Lines        9312     9317       +5     
==========================================
+ Hits         6021     6028       +7     
+ Misses       3291     3289       -2     
Impacted Files Coverage Δ
...tractions/blueprints/mixins/onboarding_required.py 74.60% <40.00%> (-2.99%) ⬇️
mephisto/data_model/unit.py 78.14% <0.00%> (ø)
mephisto/data_model/assignment.py 61.71% <0.00%> (+3.90%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ℹ️ This file allows for git to allow you to commit an empty folder. This assets folder is important for the tips cypress tests.
Copy link
Contributor

@JackUrb JackUrb left a comment

Choose a reason for hiding this comment

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

I love this feature!

The one thing I'd suggest though is that people using this may also want to be able to change/query by worker_name rather than just by the id, as they may know the former and not the latter. Still, this is a great feature to have either way.

@Etesam913
Copy link
Contributor Author

I changed it to query by worker name.

I added a large comment in the script as what I am doing may be quite confusing.

@Etesam913 Etesam913 requested a review from JackUrb August 9, 2022 21:33
@pringshia
Copy link
Contributor

pringshia commented Aug 10, 2022

Looks fine to me, will let Jack chime in on the worker_name comment/change

Copy link
Contributor

@JackUrb JackUrb left a comment

Choose a reason for hiding this comment

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

LGTM! Agreed that the distinction between mephisto worker ids and worker names (which are usually the external IDs for a worker) is confusing. Unsure how to improve this at the moment.

@Etesam913
Copy link
Contributor Author

The simplest solution to fix this would probably to rename "worker_id" to "worker_name" in the url, but that can be done in a separate pr.

@Etesam913 Etesam913 merged commit 6df02d6 into main Aug 10, 2022
Sprint #15 - Affectionate Moose (Aug 15) automation moved this from In progress to Done Aug 10, 2022
@JackUrb JackUrb deleted the reset-worker-onboarding branch September 14, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Development

Successfully merging this pull request may close these issues.

Utility to reset worker's onboarding status
5 participants