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

Enhancement : Require mentor's approval for task completion #1130

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

RiddhiAthreya
Copy link
Contributor

Description

Task has got new property requires_approval: [bool]. If it's set to True during creation, task becomes "restricted" - i.e only mentor can mark it as done/completed. When mentee tries to mark it as done, he get's 401 Unauthorized.

Fixes #278

Type of Change:

  • Code

Code/Quality Assurance Only

  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

When a mentee tries to update a task which has requires_approval set to True
Screenshot (190)

creating a task with required_approval set to True
Screenshot (191)

When mentor tries to update a task which has requires_approval set to True
Screenshot (192)

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Update Postman API at /docs folder
  • Update Swagger documentation and the exported file at /docs folder
  • Update requirements.txt

Code/Quality Assurance Only

  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@epicadk
Copy link
Member

epicadk commented Jul 6, 2021

You'll also need to add migrations

@RiddhiAthreya
Copy link
Contributor Author

@epicadk can you please tell me how to add that?

@epicadk
Copy link
Member

epicadk commented Jul 6, 2021

@epicadk can you please tell me how to add that?

I believe @mtreacy002 was going to create a doc for this?

@mtreacy002
Copy link
Member

You'll also need to add migrations

@epicadk , IMO @RiddhiAthreya doesn't need to write a migration script here since the changes applies to dao object of Task and the logic used inside Task db model. The Task db model itself doesn't get affected with the changes here. I tried running flask db migrate and got the response below.

Screen Shot 2021-07-08 at 3 30 36 pm

Let me know if you have a different view on this 😉

@RiddhiAthreya
Copy link
Contributor Author

@mtreacy002 true , only the task json object has changed, but the task table remains the same 😃

@vj-codes vj-codes added the Status: Needs Review PR needs an additional review or a maintainer's review. label Jul 11, 2021
Copy link
Member

@devkapilbansal devkapilbansal left a comment

Choose a reason for hiding this comment

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

@RiddhiAthreya I see this will work but how the mentor is going to approve the task??

Also, we need some test cases for this too

Comment on lines +33 to +36
if 'requires_approval' in data.keys():
requires_approval = data['requires_approval']
else:
requires_approval = False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if 'requires_approval' in data.keys():
requires_approval = data['requires_approval']
else:
requires_approval = False
requires_approval = data.get('requires_approval') or False

@devkapilbansal devkapilbansal added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Jul 25, 2021
@devkapilbansal
Copy link
Member

@RiddhiAthreya any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Require mentor's approval for task completion
5 participants