Skip to content

62 rudimentary photo submission into database - #296

Open
schen017 wants to merge 16 commits into
mainfrom
62-rudimentary-photo-submission-into-database
Open

62 rudimentary photo submission into database#296
schen017 wants to merge 16 commits into
mainfrom
62-rudimentary-photo-submission-into-database

Conversation

@schen017

@schen017 schen017 commented Feb 4, 2025

Copy link
Copy Markdown

Pull Request

Brief Summary

  • Wrote POST /photos/upload/url to allow photographers to upload photos by link (URL).
  • Wrote addPhotoByURL in PhotoController.
  • Wrote createPhoto in PhotoAccessor.
  • Updated getPhotoByID in PhotoAccessor to populate photographers and tags (in order to create a PhotoResponse in addPhotoByURL in PhotoController).
  • Wrote getTagsByName in PhotoTagAccessor in order to fetch the tags in addPhotoByURL to create a new photo.
  • Updated PhotoCreate in apiModels. Updated the tags and photographers fields to be an array of strings.
  • Updated PhotoSchema in dbModels. Updated tags to reference PhotoTag and photographers to reference User to allow tags and photographers to be populated.
  • Did not write file upload.

Questions / Considerations for the Future

  • We referenced and used the already defined schemas in the code. Therefore, photos don’t have the fields “internalPhotographer” and “license”, and include the field “rights.”
  • We don’t test that the photo tags are correct. Should we create new photo tags in testData?
  • Are photo tags required (or can the array be empty)?

API Changes

  • Wrote POST /photos/upload/url.

Database Changes

  • Updated PhotoSchema in dbModels. Updated tags to reference PhotoTag and photographers to reference User to allow tags and photographers to be populated.

New Tests

  • Wrote 3 tests for valid photo uploads by URL.

Closes #62

…or photos, added tests for the functionality
…fix) and fetched objects from db in photoController instead of strings

@ethanszeto ethanszeto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You did good for quite a complicated ticket with semi-unclear directions (thats on me). Just a couple things to clean up here and there, and the failing tests, but it looks like most of the code is written.

Comment thread app/controllers/photoController.js Outdated
Comment on lines +25 to +28
req.body.creationTime = req.body.creationTime ? new Date(req.body.creationTime) : req.body.creationTime;
req.body.modificationTime = req.body.modificationTime
? new Date(req.body.modificationTime)
: req.body.modificationTime;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CreationTime and ModificationTime should be automatically added to the object during Validation. Look into the JSON Schema (validationSchema) to build a validator for you to accomplish this.

Comment thread app/controllers/photoController.js Outdated
? new Date(req.body.modificationTime)
: req.body.modificationTime;
// create a PhotoCreate object from the request
const photoCreate = new PhotoCreate(req.body);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

API Models is deprecated for validation (Sorry, was not clear initially on this)

Comment thread app/controllers/photoController.js Outdated
Comment on lines +31 to +52
// fetch users
const fetchUsers = async (emails, role) => {
const users = await UsersAccessor.getUsersByEmail(emails);
if (users.length !== emails.length) {
throw new ErrorInvalidRequestBody(`Invalid ${role} emails`);
}
return users;
};
// fetch tags
const fetchTags = async (tagNames) => {
const tags = await PhotoTagAccessor.getTagsByName(tagNames);
if (tags.length !== tagNames.length) {
throw new ErrorInvalidRequestBody(`Invalid tag names`);
}
return tags;
};
const { url, tags, photographers, photoTime, rights, creationTime, modificationTime } = photoCreate;
// fetch the photographers
const photographerUsers = await fetchUsers(photographers, "photographers");
// fetch the photo tags
const photoTags = await fetchTags(tags);
// create a new photo using the photographers and photo tags

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In order to get the MongoIds of the users/tags/etc, I believe there are helper methods available. For PhotoTag, you might have to create that helper method yourself.

const tagNamesToIds = (
    [/*list of photo tag names*/]
) => {
    [/*list of photo tag ids*/]
}

Comment thread app/controllers/photoController.js Outdated
Comment on lines +55 to +56
tags: photoTags,
photographers: photographerUsers,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remember that, although the populated record will return every detail, when you are inserting into the database, these references are actually just the _id's of the objects, not the whole objects themselves.

You can think of tags and photographers as foreign keys to another record that exists in a different collection.

Comment thread app/controllers/photoController.js Outdated
Comment on lines +68 to +69
// validation needed?
// return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can add outgoing validation here yes

Comment on lines +17 to +21
static async createPhoto(photo) {
await Connection.open();
const createPhoto = await Photo.create(photo);
return createPhoto;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good

Comment on lines +42 to +49
const photo = await Photo.findById(new mongoose.Types.ObjectId(photoID))
.populate("photographers")
.populate({
path: "tags",
populate: {
path: "creatingUser",
},
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

Comment on lines +65 to +73
try {
await Connection.open();
const tags = await PhotoTag.find({ tagName: { $in: tagNames } });
return tags;
} catch (error) {
console.error(error);
throw new Error(error);
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Validation/Error handling should exist in the controller level, not the accessor

Comment on lines +14 to +15
tags: { type: [string], required: true },
photographers: { type: [string], required: true },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch

Comment thread app/models/apiModels/photo.js Outdated
Comment on lines +2 to +3
// import Photography_status from "../enums/photography_status.js";
//import { ErrorInternalAPIModelValidation } from "../../error/internalErrors.js";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If unused, you can remove these

@ethanszeto

Copy link
Copy Markdown
Member
  • We referenced and used the already defined schemas in the code. Therefore, photos don’t have the fields “internalPhotographer” and “license”, and include the field “rights.”

Thats fine.

  • We don’t test that the photo tags are correct. Should we create new photo tags in testData?

You can, not part of the scope of the ticket, however, it may prove to be useful for someone in the future.

  • Are photo tags required (or can the array be empty)?

Phototags are not required when submitting/uploading a photo

…a to validate photo creation. did not modify tests
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.

Rudimentary Photo Submission into Database

3 participants