translators icon indicating copy to clipboard operation
translators copied to clipboard

Update call number field in Primo Normalized XML

Open malkaPre opened this issue 1 year ago • 10 comments

Before fix: Call-number field was taken from '//p:display/p:availlibrary' root in the pnx or '//p:browse/p:callnumber' root in the pnx and that matched primo pnx format. In primoVE the call-number field exists in a differnt root '//p:delivery/p:bestlocation/p:callNumber' in the pnx.

After fix: if call-number is empty it will fetch from '//p:delivery/p:bestlocation/p:callNumber'.

malkaPre avatar Jul 09 '23 06:07 malkaPre

Hello, thank you for your first pull request :)

A few comments -

  • Please update the translator files using Scaffold (Zotero -> "Tools" menu -> Developer -> Translator Editor). When you save using Scaffold, it will update the timestamp in the metadata. Without the timestamp update, update to the code will not be picked up by Zotero.
  • Could you provide an example where it used to fail without the change, which is corrected by the change? Have you tried running the tests in Scaffold? If necessary, please consider adding a test. (More on Scaffold: https://www.zotero.org/support/dev/translators/scaffold)
  • Typically there's no need to merge the upstream changes into your branch. You can just focus on adding commits to your branch and push them, so only the affected file shows up in the commits.

zoe-translates avatar Jul 11 '23 11:07 zoe-translates

In this URL when you save to Zotero and show empty fields, call number field is empty: https://quicksearch.lib.iastate.edu/discovery/fulldisplay?docid=alma9920251487302756&context=L&vid=01IASU_INST:01IASU&lang=en&search_scope=MyInst_and_CI&adaptor=Local%20Search%20Engine&tab=Everything&query=any,contains,fred&offset=0

malkaPre avatar Jul 12 '23 13:07 malkaPre

Is there any updates on this?

malkaPre avatar Jul 20 '23 08:07 malkaPre

See comments from @zoe-translates and me

AbeJellinek avatar Aug 24 '23 15:08 AbeJellinek

@AbeJellinek I checked all the comments - and I did whatever was requested:

  1. Updated the translator files using Scaffold.
  2. Gave an example where it used to fail without the change.
  3. You agreed that the following syntax is okay. https://github.com/zotero/translators/pull/3075#discussion_r1262942934 image

Am I missing out on something? If yes, I would like you to explain to me what.

malkaPre avatar Aug 27 '23 08:08 malkaPre

The customer is requesting for updates. Can you please review and approve the change being requested

malkaPre avatar Apr 04 '24 07:04 malkaPre

  • You reverted the lastUpdated change.
  • I didn’t say that the regex was OK. I said that I agreed with @zoe-translates’ comments about the regex being confusing. It is confusing! And we shouldn’t be using search().
  • You didn’t add a test.

AbeJellinek avatar Apr 04 '24 12:04 AbeJellinek

Hi @AbeJellinek @zoe-translates

We would like your help in closing this issue, since we have Primo customers that are waiting for this fix. @malkaPre is encountering with issues while testing using the Scaffold.

We have already implemented your remarks on the regex. please help to optimize the process as we do want to commit the change and provide the fix.

please suggest if possible options to help and speed this up.

All the best Nili Natan Primo, Product Manager [email protected]

nilinatan avatar Jul 15 '24 11:07 nilinatan

@malkaPre is encountering with issues while testing using the Scaffold.

What are those issues?

We have already implemented your remarks on the regex.

No, you did not. Or if you did, you haven't pushed it.

please help to optimize the process as we do want to commit the change and provide the fix.

What do you mean?

AbeJellinek avatar Jul 24 '24 13:07 AbeJellinek