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.

Sign up for GitHub

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

Jump to bottom

visit-REST-delete endpoint new implementation #234

Merged
merged 2 commits into from
Dec 13, 2023
Merged

visit-REST-delete endpoint new implementation #234

merged 2 commits into from
Dec 13, 2023

Conversation

odis43
Copy link
Contributor

@odis43 odis43 commented Dec 8, 2023

Notion ticket link

Ticket Name

Implementation description

  • delete endpoint for visits (attendance sheets and records)

Steps to test

  1. if deletion works
  2. if deletion on invalid requests work
  3. deletion on items not yet in the db are handled

What should reviewers focus on?

  • as above

Checklist

  • [ x] My PR name is descriptive and in imperative tense
  • [ x] My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • [ x] I have run the appropriate linter(s)
  • [x ] I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

@odis43 odis43 requested a review from Ezzhingy December 8, 2023 18:53
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 8, 2023
View reviewed changes
backend/python/app/rest/visit.py
attendance_sheet_service.delete_attendance_sheet(id)
return f"Attendance sheet and record with id: {id} has been deleted", 200
except Exception as error:
return jsonify({"error": str(error)}), 500

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
Loading
flows to this location and may be exposed to an external user.
Ezzhingy
Ezzhingy approved these changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@Ezzhingy Ezzhingy left a comment

Choose a reason for hiding this comment

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

lgtmm

@odis43 odis43 merged commit 93f33c8 into main Dec 13, 2023
5 checks passed
@odis43 odis43 deleted the odi/visit-REST-delete-new branch December 13, 2023 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ezzhingy Ezzhingy Ezzhingy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@odis43 @Ezzhingy