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: POST/GET/PUT program and list of programs #114
Feat: POST/GET/PUT program and list of programs #114
Conversation
Update @anitab-org/bridgeintech-maintainers . I've completed the POST/GET/PUT program functionalities. Note that atm I only GET endpoint for Programs list using
NOTE I will write the test cases for these PR endpoints on a separate PR as this PR already pretty big. Please review when you have time. Thanks |
app/api/dao/program.py
Outdated
return user.message, user.status_code | ||
|
||
programs = ProgramModel.get_all_programs_by_organization(organization_id) | ||
list_of_programs = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not declare list of programs after “if not programs:”
Also, lines 69-70 can just be a list comprehension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Changed now
|
||
def update(program, data, success_message, status_code): | ||
target_candidate_data = {} | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some info should be given if it is a bad request, such as which field(s) is malformed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramitsawhney27 , I do it this way coz for the fields that required specific format, the validation already done specific to each fields in the preceding code (inside app/api/resources/organizations.py) line 105
as for the TypeError e
done here, will specify which fields other than those catch in the above that is malformed. Let me know if you have another alternative. Thanks
app/api/validations/organization.py
Outdated
if not is_phone_valid(mobile): | ||
return messages.PHONE_OR_MOBILE_IS_NOT_IN_NUMBER_FORMAT | ||
except ValueError as e: | ||
return e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is e being returned? Can you add a comment to the code to explain this to future readers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Refactored email, phone and mobile to LBYL instead of EAFP as the LBYL makes more sense in this situation and use less lines of codes
e37ad31
to
d10a27a
Compare
Update @anitab-org/bridgeintech-maintainers, I've pushed the changes requested by @ramitsawhney27 . Can you please check again? Thanks |
app/api/dao/program.py
Outdated
programs = ProgramModel.get_all_programs_by_organization(organization_id) | ||
if not programs: | ||
return messages.NO_PROGRAM_FOUND, HTTPStatus.NOT_FOUND | ||
list_of_programs = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 68-70 (even 71) can be condensed into 1 line using list comprehension. Could you please do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, @ramitsawhney27 . It surely looks better now. Thanks heaps for the tips. 😉
d10a27a
to
661f41d
Compare
Refactor validations refactor list_of_programs to use list comprehension Remove is_organization_rep being returned upon login
661f41d
to
561b728
Compare
Update @ramitsawhney27 and @anitab-org/bridgeintech-maintainers . I've removed the is_organization_rep from being returned when the user login as per @meenakshi-dhanani advice here. Can you please re-review this PR? Thanks |
Paired with @mtreacy002 to test this! LGTM! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future reference,
This is a test case for PUT /organization
:
{
"representative_department": "momo",
"name": "moasdea",
"email": "maomo@test.com",
"about": "moamo",
"address": "123 test street",
"website": "string",
"timezone": "America/New_York",
"phone": "123-445-232",
"status": "Publish"
}
This is a test case for POST /organizations/{organization_id}/programs/program
:
{
"program_name": "Enhance soft skill program",
"start_date": "2020-08-25 00:00",
"end_date": "2020-09-22 11:11",
"description": "come join us ",
"target_skills": [
"Public Speaking"
],
"target_candidate_gender": "Not Applicable",
"target_candidate_age": "Not Applicable",
"target_candidate_ethnicity": "Not Applicable",
"target_candidate_sexual_orientation": "Not Applicable",
"target_candidate_religion": "Not Applicable",
"target_candidate_physical_ability": "Not Applicable",
"target_candidate_mental_ability": "Not Applicable",
"target_candidate_socio_economic": "Not Applicable",
"target_candidate_highest_education": "Not Applicable",
"target_candidate_years_of_experience": "Not Applicable",
"target_candidate_other": "",
"payment_currency": "USD",
"payment_amount": 0,
"contact_type": "Face-to-face",
"zone": "Global",
"student_responsibility": [
"string"
],
"mentor_responsibility": [
"string"
],
"organization_responsibility": [
"string"
],
"student_requirements": [
"string"
],
"mentor_requirements": [
"string"
],
"resources_provided": [
"string"
],
"contact_name": "nicole",
"contact_department": "momo department",
"program_address": "123 test street",
"contact_phone": "123-456-333",
"contact_mobile": "123-333-333",
"contact_email": "1sdas@df.com",
"program_website": "https://123web.com",
"irc_channel": "momo123",
"tags": [
"career"
],
"status": "Open"
}
Description
Added create, gt and update programs using the following endpoints:
The purpose of this endpoint is so on the frontend, when user go to
Organizations
> 'Organization-Portfolo`, they can see the list of programs that organization have. Any user can retrieve this list.The purpose of this endpoint is so that logged-in user can view the details on a specific program after selecting it from the list of programs from API endpoint number 2 above.
Fixes #113
Type of Change:
Code/Quality Assurance Only
How Has This Been Tested?
Note: The screenshots below are selected from random data
Checklist:
Code/Quality Assurance Only