CGUC / skybunk-server

The back-end application for Conrad Grebel University College students to stay connected
3 stars 9 forks source link

Add more generic 'Media' object to posts model (Open to comments) #89

Closed picklechips closed 5 years ago

picklechips commented 5 years ago

The idea with this PR is to generalise media types on posts. "Media" meaning images, polls, videos, etc.

This allows our types of posts to be much more easily extendable, since all you have to do is create a model for the new type and add it into the Media model.

As an example, in the future if we want to add polls to posts we would just create a "Poll" model, and then add the following code snippet into the Media model:

poll: {
    type: Schema.Types.ObjectId,
    ref: 'Poll',
    validate: mediaValidator.bind(this)('poll')
}

Add a wee bit of creation logic, and then you're done. Wouldn't even have to touch the Posts model.

Then on the client-side, we could simply check the type of media on the post and then display it appropriately.

You can check out the tests to get somewhat of an idea of the usage. Looking for thoughts or opinions, curious as to what others think.

scholvat commented 5 years ago

Seems like a good organization structure, though getting this to be backwards compatible might be a challenge. I would think a script would also need to be written to convert all posts to this new schema.

picklechips commented 5 years ago

It would, but I wouldn't worry about that until after it's all merged in. I think the correct way about doing this is the following:

picklechips commented 5 years ago

This has been updated. @scholvat @aopal

I've also opened another PR for the polls model, which demonstrates how this media object would be used.

https://github.com/picklechips/skybunk-server/pull/1

picklechips commented 5 years ago

@scholvat @aopal @neilbrub I'll be merging this tomorrow sometime if there's no other comments!