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

ExportFeed visual editor creating invalid definitions #4650

Open
grantfitzsimmons opened this issue Mar 14, 2024 · 2 comments · May be fixed by #4660
Open

ExportFeed visual editor creating invalid definitions #4650

grantfitzsimmons opened this issue Mar 14, 2024 · 2 comments · May be fixed by #4660
Assignees
Labels
1 - Bug Incorrect behavior of the product 2 - App Resources Issues that are related to app resources 2 - Exporting Data Issues that are related to exporting data to DwC, GBIF, IPT, Web Portal, etc.
Milestone

Comments

@grantfitzsimmons
Copy link
Contributor

Describe the bug
Creating or modifying an ExportFeed app resource using the visual editor results in export failures due to a KeyError in the code. The issue stems from attribute naming inconsistencies (i.e. collectionid instead of collectionId). To ensure functionality, the visual editor must use consistent camelcase for attribute names in line with existing resources.

New ExportFeed app resource created with the visual editor

This is not working.

<?xml version="1.0" encoding="UTF-8"?>
<channel>
	<title>Test</title>
	<description>Test</description>
	<item collectionid="4" userid="1" notifyuserid="1" definition="DwCA_voucher" metadata="DwCA_voucher_metadata" days="7" filename="kudwca.zip" publish="true">
		<title>KU Fish Voucher</title>
		<id>ba7b743e-3263-4016-aebf-995f0d6a6e19</id>
	</item>
</channel>

After creating or adding to an existing ExportFeed app resource using the visual editor in edge, all exports will fail. The errors look like this on the several databases I tested this on:

"traceback":"Traceback (most recent call last):\n  File \"/opt/specify7/specifyweb/export/views.py\", line 169, in try_update_feed\n    update_feed(force=True, notify_user=user)\n  File \"/opt/specify7/specifyweb/export/feed.py\", line 53, in update_feed\n    collection = Collection.objects.get(id=item_node.attrib['collectionId'])\nKeyError: 'collectionId'\n"}

This error indicates a "KeyError" occurred in the code, specifically when trying to access a key ('collectionId') in a dictionary where this key is not present. That led me to look and see if this key is being created by the visual editor, and lo and behold, it is not.

The visual editor is creating a collectionid attribute in <item> rather than collectionId (notice the case difference).

Existing ExportFeed app resource created manually

<channel>
  <title>KUBI ichthyology RSS Feed</title>
  <description>RSS feed for KUBI Ichthyology Voucher and Tissue collections</description>
  <item collectionId="4" userId="2" notifyUserId="2" definition="DwCA_voucher" metadata="DwCA_voucher_metadata" days="7" filename="kui-dwca.zip" publish="true">
    <title>KU Fish</title>
    <id>8f79c802-a58c-447f-99aa-1d6a0790825a</id>
  </item>
</channel>

Notice collectionid is collectionId, userid is userId, notifyuserid is notifyUserId, and all the other names are consistent.

The visual editor needs to use the correct keys, so it should use camelcase properly to make it consistent with existing ExportFeed resources. If we shipped everything now, users would be unable to create working export feeds and be unable to use the "Update RSS Feed" function after doing so.

@grantfitzsimmons grantfitzsimmons added 1 - Bug Incorrect behavior of the product 2 - App Resources Issues that are related to app resources 2 - Exporting Data Issues that are related to exporting data to DwC, GBIF, IPT, Web Portal, etc. labels Mar 14, 2024
@grantfitzsimmons grantfitzsimmons added this to the 7.9.4 milestone Mar 14, 2024
@CarolineDenis CarolineDenis modified the milestones: 7.9.4, 7.9.6 Mar 14, 2024
@CarolineDenis
Copy link
Contributor

CarolineDenis commented Mar 14, 2024

Attribute converted to lower case happens in:

handleChange(() => updateXml(deserializer(definition)));

more specially in deserializer(definition)

@CarolineDenis CarolineDenis self-assigned this Mar 15, 2024
@CarolineDenis CarolineDenis linked a pull request Mar 15, 2024 that will close this issue
8 tasks
@maxpatiiuk
Copy link
Member

maxpatiiuk commented Mar 16, 2024

When I was writing syncer, I looked at every single form attribute that ever existed, and they were all lowercase (I forgot to look at Export Feed). To catch typos, on the front-end we always use camelCase instead of lowercase (so notifyUserId instead of notifyuserid - more readable, and less bug-prone as IDE can catch typos), so I made syncer allow you to specify attribute in camel case, then it would look for camel case or lower case attribute in XML when reading, but convert to lower when writing back.

Given that export feeds is the only place where camelCase XML attributes are used, and that export feeds are only used by sp7, not sp6, I would say a propper fix for this is to make sp7 back-end more case-insensitive, rather than modifying the syncer (though we could add a flag to xmlAttribute to disable automatic lowercase conversion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - Bug Incorrect behavior of the product 2 - App Resources Issues that are related to app resources 2 - Exporting Data Issues that are related to exporting data to DwC, GBIF, IPT, Web Portal, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants