Skip to content

#201 delete photo tag (Fetch more recent codebase)#295

Open
dsyamy wants to merge 2 commits intomainfrom
201-PhotoTag-Delete-Route-Updated
Open

#201 delete photo tag (Fetch more recent codebase)#295
dsyamy wants to merge 2 commits intomainfrom
201-PhotoTag-Delete-Route-Updated

Conversation

@dsyamy
Copy link
Contributor

@dsyamy dsyamy commented Feb 4, 2025

Old branch wasn't letting me fetch new codebase so had to make a new one.

Pull Request

Brief Summary

  • Fixed the path for the photo model as it was causing issues.
  • authorization of tag deletion is given to admin and photographers.
  • Included the cascading effect of photoTag deletion, but only to Photo at the moment (since it's the only model that references photoTag, but future models with references can be added to referencesToPhotoTag in photoTagAccessor.js
  • provided 3 tests: 1 proper deletion, 1 improper deletion (tag does not exist), 1 proper deletion with cascading effect

Questions / Considerations for the Future

-

API Changes

  • updated the endpoint photo-tag/delete/:tagName

Database Changes

-

New Tests

  • included new test for cascading deletion

Closes #201

Old branch wasn't letting me fetch new codebase so had to make a new one.
@dsyamy dsyamy self-assigned this Feb 4, 2025
@dsyamy dsyamy requested review from a team as code owners February 4, 2025 00:59
suttonspindler
suttonspindler previously approved these changes Feb 4, 2025
Copy link
Contributor

@suttonspindler suttonspindler left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Arushi55 Arushi55 left a comment

Choose a reason for hiding this comment

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

Looks mostly good. My only issue is the double call to the accessor seems unnecessary.

removed photographer's permission, changed delete to only use tagName, and throw error if tag doesn't exist within a nested try catch
@dsyamy
Copy link
Contributor Author

dsyamy commented Feb 10, 2025

For the ErrorPhotoTagNotFound(), I opt for a nested try-catch in the controller bc wasn't sure if throwing it in accessor would be bad design, but if not, I can edit again.

@dsyamy dsyamy requested a review from Arushi55 February 19, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PhotoTag Delete Route

4 participants