Do not allow calling some Citation object methods before adding Citation to SourceItemWrapper

[diegodlh]

As described here, calling a Citation object's linkToZoteroItem method before the Citation has been added to a SourceItemWrapper object via its addCitations method may be problematic. For example, what would happen if a Citation object is created and its linkToZoteroItem method is called, but for some reason it isn't added to a SourceItemWrapper object in the end? This may apply to other Citation object's methods.

Do we need to specify the Citation's source item at instantiation? Can we wait until the Citation has been added to a SourceItemWrapper?

[Dominic-DallOsto]

Thinking about this a bit - I don't think a citation really makes sense without a source item? Would it be best to specify the source item in the constructor, then if this is undefined/null an error will be thrown when calling a method that needs a source item?

[diegodlh]

I agree with you that a citation doesn't make sense without a source/citing item. The citation's source item is already specified in the Citation constructor. Unfortunately, I think that passing undefined or null would not work, because the constructor uses some source item's properties and methods. For example, it passes the source item's saveCitations method to the target item wrapper, and it calls the Citation's addOCI method, which in turn reads the source item's DOI, etc.

Since, as you said, a citation doesn't make sense without a source item, I think we could generally avoid using the Citation constructor, and use a source item's addCitation method instead:

Every time new Citation() is used (except inside source item's loadCitations), it is followed by a call to source item's addCitations. Decoupling (1) creation of new Citation objects from (2) actual addition to their source item may have made sense at some point to support batch operations where multiple citations are added at once (avoiding having to write the note attachment each time). However, with the introduction of source item's startBatch and endBatch methods, it is possible that this separation became no longer needed.

Maybe we could:

1. Rename the source item's addCitations method as addCitation (in singular). Have it accept an item, an array of ocis and a zotero key (like the Citation constructor does; instead of a Citation object), and make it return the Citation object created.
2. Replace all calls to new Citation() (outside source item's loadCitations and our new addCitation) and its companion call to addCitations with our new addCitation: here 1, here 2, here 3, and here 4. In 2 and 3, call source item's startBatch and endBatch before the first and after the last call to addCitation. In 4 we would need to delete the citation added if the user cancels.

What do you think of this approach?