Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patched Zotero.Utilities.Internal.itemToExportFormat prevents export of unsaved Zotero items #2926

Closed
Dominic-DallOsto opened this issue Aug 5, 2024 · 23 comments
Labels

Comments

@Dominic-DallOsto
Copy link

Dominic-DallOsto commented Aug 5, 2024

Debug log ID

RKDGUGVC-refs-euc/6.7.214.2920.6605-7

What happened?

I'm working on updating the Cita extension for Zotero 7 and noticed this error.

Cita stores data about the citations made by each Zotero item in a custom format (not as proper Zotero items). When we want to export all the citations belonging to a particular item we convert this to a list of Zotero Items that aren't actually saved in the database and pass these to the Zotero translator.

A while ago now we encountered a different error when exporting to better bibtex format, because these "unsaved" items didn't have citation keys, but we developed the workaround of first manually giving them citation keys diegodlh/zotero-cita#145

But this new issue occurs when exporting to any format (fixExportFormat is always called here when exporting items) because these unsaved items don't have ids, and so trigger the error here.

Would it be possible to first check whether the item to be serialised has a citation key and if so use it, before trying to get the item's citation key from the key manager by item ID?

I checked to make sure - exporting cited items from Cita works fine when BBT isn't installed.

(sorry that this is using an unrelated debug build of BBT - the current release 6.7.214 isn't up to date with the latest changes in item-export-format.ts)

@retorquere
Copy link
Owner

Can I test whether the item is unsaved? That would be the best thing to check for, right?

@retorquere
Copy link
Owner

Oh I can just test for the presence of id

@Dominic-DallOsto
Copy link
Author

Ahh yeah - that makes sense. Is item.id null by definition if the item is unsaved?

@retorquere
Copy link
Owner

I think it's undefined rather than null. Build incoming.

Copy link

github-actions bot commented Aug 5, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.215.2926.6662 ("Merge branch 'master' into gh-2926")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.215.2926.6662, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

@retorquere retorquere added the bug label Aug 5, 2024
@retorquere
Copy link
Owner

That should work. Ik you can verify, I'll put out a new release.

Copy link

github-actions bot commented Aug 5, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.215.2926.6663 ("cleaner this way")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.215.2926.6663, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

@Dominic-DallOsto
Copy link
Author

Dominic-DallOsto commented Aug 5, 2024

Actually these builds didn't work for me because item.id is null

For me, new Zotero.Item().id == null

Copy link

github-actions bot commented Aug 5, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.215.2926.6664 ("why null?")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.215.2926.6664, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

@Dominic-DallOsto
Copy link
Author

Perfect thanks, that works now!

@retorquere
Copy link
Owner

A release build is running, should be out in 10.

@retorquere
Copy link
Owner

This solves the primary problem (which is good, and I'm happy you caught it), but you'd have to test diegodlh/zotero-cita#145 again. Background exports will not work because it needs serialized items to be present in BBTs serialization cache, which is keyed on item id, but you can easily avoid those, and I take it you're not exporting large batches of items.

Copy link

github-actions bot commented Aug 5, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.216.2926.6667 ("sort items without itemID")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.216.2926.6667, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

Copy link

github-actions bot commented Aug 5, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.216.2926.6668 ("oops, at the end")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.216.2926.6668, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

@Dominic-DallOsto
Copy link
Author

Dominic-DallOsto commented Aug 5, 2024

This solves the primary problem (which is good, and I'm happy you caught it), but you'd have to test diegodlh/zotero-cita#145 again. Background exports will not work because it needs serialized items to be present in BBTs serialization cache, which is keyed on item id, but you can easily avoid those, and I take it you're not exporting large batches of items.

Thanks! Maybe this should be another issue, but it works to export to BBT format. But there is one issue that different items are given duplicate citation keys. Since this commit (6aa0c6d) it seems that propose doesn't have a second parameter anymore, as was used here (https://github.com/diegodlh/zotero-cita/pull/147/files). Is there an updated way of handling this?

For example I end up with the following after export:

@article{smithPaper2022,
  title = {Paper},
  author = {Smith, J},
  year = {2022},
  journal = {J2}
}

@article{smithPaper2022,
  title = {Paper},
  author = {Smith, J},
  year = {2022},
  journal = {J1}

@retorquere
Copy link
Owner

Sorry about that. New build incoming with this change.

@retorquere
Copy link
Owner

Note it's a Set now, not an array.

@retorquere
Copy link
Owner

Is that confirmation that it works? If so, I'll put out another release.

@Dominic-DallOsto
Copy link
Author

Is that confirmation that it works? If so, I'll put out another release.

No I was just speculating. I can't see the new build yet?

Copy link

github-actions bot commented Aug 6, 2024

🤖 this is your friendly neighborhood build bot announcing test build 6.7.216.2926.6673 ("pandoc")

This update may name other issues, but the build just dropped here is for you; it just means problems already fixed in other issues have been folded into the work we are doing here. Install in Zotero by downloading test build 6.7.216.2926.6673, opening the Zotero "Tools" menu, selecting "Add-ons", open the gear menu in the top right, and select "Install Add-on From File...".

@Dominic-DallOsto
Copy link
Author

Perfect thanks! Now I get different keys.

@article{smithPaper2022a,
  title = {Paper},
  author = {Smith, J},
  year = {2022},
  journal = {J1}
}

@article{smithPaper2022,
  title = {Paper},
  author = {Smith, J},
  year = {2022},
  journal = {J2}
}

@retorquere
Copy link
Owner

Release build is running.

@Dominic-DallOsto
Copy link
Author

Yep! Works all good with 6.7.217. Thanks a lot!

@github-actions github-actions bot reopened this Aug 6, 2024
@github-actions github-actions bot removed the reopened label Aug 6, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants