search

pepyatka / pepyatka-server

Server for Pepyatka
79 stars 27 forks source link

Add support for editing attachments #292

Closed clbn closed 8 years ago

clbn commented 8 years ago

@epicmonkey @indeyets @berkus

Please take a look at the implementation of unlinkAttachments() in the change. I need a confirmation that this piece of code does a proper DB change:

database.lremAsync(mkKey(['post', that.id, 'attachments']), 0, attachmentId),
database.hsetAsync(mkKey(['attachment', attachmentId]), 'postId', '')

(as an opposite to what we do in its sister method, linkAttachments()).

epicmonkey commented 8 years ago

I'd suggest using hdel instead of hset to an empty value.

clbn commented 8 years ago

@epicmonkey Thank you, it makes sense.

On the other hand, when we create an attachment, it has postId === '', so hset with an empty string just restores it to the "original" unattached state.

epicmonkey commented 8 years ago

Doh! :-)

clbn commented 8 years ago

So should I change it or leave as it is now? Gimme my +1!

clbn commented 8 years ago

@berkus Parameter name has been fixed.

clbn commented 8 years ago

@berkus @indeyets @epicmonkey Ping.

epicmonkey commented 8 years ago

other than Post.prototype.unlinkAttachments introduces a new promise (vs es6 await) LGTM.

clbn commented 8 years ago

@epicmonkey Basically just copied it from linkAttachments(). Didn't want to add refactoring to this change, just to be on the safe side.