Import a single Trello card (and its comments)

Brought to you by: wekan

#358 Import a single Trello card (and its comments)

Status: closed

Owner: nobody

Labels: None

Updated: 2015-10-14

Created: 2015-10-14

Creator: Anonymous

Private: No

Originally created by: xavierpriour

This is step 1 of [#113]
We import a single card, with its title, description, comments, and labels.
Limitations (to be fixed in future PRs):

card members are removed
Trello history is lost
attachments are not transferred

Discussion

Anonymous - 2015-10-14

Originally posted by: xavierpriour

Oops forgot to eslint - will fix!

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

merged with core wekan/devel + fixed an eslint issue in one of the new files - now passing :)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

Hi Xavier,

Thank you for the PR! The code looks good to me. Apart from my tiny inline comments there is one bigger change I would like to consider with you before merging this PR. I don’t know how much you are familiar with Meteor, but basically there are two paths to mutate the database, one is using Meteor.methods that are basically RPC (but with potential optimistic update on the client) and the other is using minimongo operations (like we everywhere in Wekan). The thing is that with minimongo .insert we enforce some constants, for instance we ensure that the createdAt date is correct (here) but in the case of the card import it is probably better to trust the data we are importing (or at least ensure a set of different rules that we would normally do). So I propose to do the import in a Meteor Method, because it makes it easy to chose which validation rule we want to enforce.

Does that makes sense?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

Perfect sense, and addresses some of my concerns mentioned in [#113]
What file should I put my method definition in, though?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

I guess models/cards.js — but we’ll probably want to split this file later on.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

Well, I'm going to start work on importing boards next - so mind if I create a models/trello.js right now and put the method in it?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

Ok, but name if import.js because at some point we’ll be able to import data exported from Wekan as well :-)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

Sure!

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

(Or even [#152] :-))

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

When generating activities for the card import itself, should it generate an 'importCard' only, or an 'importCard' + a 'createCard' (with same date)?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

I think only “importCard” will be less confusing.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

@mquandalle it should be good now

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

Tested and it works well! Thank you Xavier, I’m ready to merge. Maybe could you just git rebase -i 21dcd16 and squashes some of the commits (for instance “fix eslint style”) and git push -f so we get a cleaner git history?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

ok, did some squashing.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: xavierpriour

ok, something seems wrong, rebase brought your commits in my PR... how do I get them out?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

Yep something went wrong, I'll fix and merge.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Originally posted by: mquandalle

Merged in b670a1ab36fcf514751e4b356487a96088dbbd24, thanks!

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2015-10-14

Ticket changed by: mquandalle

status: open --> closed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Log in to post a comment.