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

feat: create GET/mentorship_relations API #168

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

khushishikhu
Copy link
Member

@khushishikhu khushishikhu commented Oct 3, 2020

Description

added GET/mentorship_relations API endpoints

Fixes #162

Type of Change:

  • Code
  • User Interface
  • Documentation

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
  • Any dependent changes have been merged

Copy link
Member

@mtreacy002 mtreacy002 left a comment

Choose a reason for hiding this comment

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

@khushishikhu , it's best to create a new file for this task (you can call it mentorship_relations.py) instead of using the users.py. Also, please check my comment below.

per_page = request.args.get("per_page", default=DEFAULT_USERS_PER_PAGE, type=int)


return http_response_checker(get_request("/users/verified", token))
Copy link
Member

Choose a reason for hiding this comment

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

why is this retuning verified users and not mentorship rrelations the login user?

@khushishikhu
Copy link
Member Author

@mtreacy002 i think we need to add the file mentorship_relation.py in the folder dao , resources and models

@khushishikhu
Copy link
Member Author

khushishikhu commented Oct 8, 2020

Capture
@mtreacy002 i just need a help, i don't know why i can't see the "mentorship relation" endpoint on the page. Through the changes i have made

@mtreacy002
Copy link
Member

@khushishikhu , you also need to add the new mentorship_relation to the namespace so you can use it as @mentorship_relations_ns in the route, just like we did to users_ns and organization_ns.

Screen Shot 2020-10-14 at 2 18 10 pm

Screen Shot 2020-10-14 at 2 25 22 pm

@mtreacy002 mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. hacktoberfest-accepted Issue that is accepted to the Hactoberfest Type: Enhancement New feature or request. labels Oct 14, 2020
@mtreacy002
Copy link
Member

@khushishikhu , can you please give me an update on this issue?

@khushishikhu
Copy link
Member Author

@mtreacy002 sorry for not updating you, i am still working on the issue , will update you in 2-3 days

@mtreacy002
Copy link
Member

@mtreacy002 sorry for not updating you, i am still working on the issue , will update you in 2-3 days

No worries, @khushishikhu. Let me know if you need any help 😉.

@khushishikhu
Copy link
Member Author

Capture
@mtreacy002 i have done the changes please review it now. and help me to know what else is needed. thank you.

@mtreacy002
Copy link
Member

Thanks, @khushishikhu . I'll review this PR today

Copy link
Member

@mtreacy002 mtreacy002 left a comment

Choose a reason for hiding this comment

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

@khushishikhu. There are a couple of things need to be modified on this PR. Also, just want to make sure you understand the task involve in this PR.

  • BIT is using existing mentorship_relation of MS System when creating a relation between program and mentor/mentee. Along with this, BIT also create an instance of mentorship_relations_extension which holds that specific information related to BIT that MS doen't have in its mentorship_relation table. When getting information on mentorship relation, the GET /mentorship_relations request will be sent to MS backend, and the response will be matched to the list of objects on mentorship_relations_extension so that an array of objects that contains info on both tables can be returned to user.
    At the moment, the code you provided here are duplication of MS backend for mentorship relation.
  • the focus of this PR is on GET mentorship_relations endpoint, not creating the mentorship relation itself. Since the issue Feat: create POST /organizations /{organization_id} /programs/{program_id} /send_request #161 is still in progress (meaning you won't be able to show HTTPStatus.OK on the GET request) you can still work on error responses first like Not Found, Internal Server Error or Unauthorized responses.

Let me know if you need any help.

@mentorship_relation_ns.marshal_list_with(mentorship_request_response_body)
def get(cls):
"""
Lists all mentorship relations of current user.
Copy link
Member

Choose a reason for hiding this comment

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

can you please give a more informative description on what the endpoint is about. You can use similar style on User's GET /users endpont below.
Screen Shot 2020-10-27 at 1 24 19 pm

Points to show:

  • the need of valid token
  • the type of input
  • what's being returned and what information are given.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thankyou for letting me i was just confused for what needs to show

MAXIMUM_MENTORSHIP_DURATION = timedelta(weeks=24) # 6 months = approximately 6*4
MINIMUM_MENTORSHIP_DURATION = timedelta(weeks=4)

def create_mentorship_relation(self, user_id: int, data: Dict[str, str]):
Copy link
Member

@mtreacy002 mtreacy002 Oct 27, 2020

Choose a reason for hiding this comment

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

There is no need to duplicate the DAO functionality already existed in Mentorship System Backend for creating mentorship_relation instance. You only need to send GET request to the MS Backend for this. On the BIT side you need to create the dao functionality related to Mentorship Relation Extension table that belongs to bitschema (hence, the name of the file also will reflect the content, mentorship_relation_extension.py - under the dao folder only). Although, the function you need to focus on this PR is only the get functionality (not create) since the create function will be dealt with in other issue (#161).

@khushishikhu
Copy link
Member Author

@mtreacy002 sorry i just got busy in some other work. i will give you an update on this at the EOD.

@khushishikhu
Copy link
Member Author

@mtreacy002 i have made the error responses like Internal Server Error or Unauthorized whereas right now i am not able to show HTTPStatus.OK (as that will depend on the completion of issue #161)

@mtreacy002
Copy link
Member

Sorry for the late reply, @khushishikhu. Sure, I'll check it out. If need to, I will create a separate PR for status OK which will be made available when issue #161 is finalised and will just review this PR based on the specific error responses you've created here. Will let you know.

@mtreacy002 mtreacy002 added the Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. label Jan 26, 2021
@mtreacy002 mtreacy002 added this to In Progress in BIT Roadmap Apr 5, 2021
@mtreacy002 mtreacy002 moved this from In Progress to Code Review in BIT Roadmap Apr 5, 2021
@mtreacy002 mtreacy002 removed this from Code Review in BIT Roadmap Apr 5, 2021
@mtreacy002 mtreacy002 added this to In Progress in BIT Roadmap via automation Apr 5, 2021
@mtreacy002 mtreacy002 moved this from In Progress to Code Review in BIT Roadmap Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. hacktoberfest-accepted Issue that is accepted to the Hactoberfest Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. Type: Enhancement New feature or request.
Projects
BIT Roadmap
Code Review
Development

Successfully merging this pull request may close these issues.

Feat: create GET /mentorship_relations API endpoint
2 participants