Added GN parameter options - group, assignToCatalog, transformwith. #148
Conversation
There was a problem hiding this comment.
Thanks for the PR @ByronCinNZ! I am happy to see that the refactoring I did a while ago apparently pays off, because you got everything right. 🙂
Some small comments, but otherwise good work! 👍
| 'group': (None, self.group, 'text/plain'), | ||
| 'assignToCatalog': (None, self.assignCat, 'text/plain'), | ||
| 'transformWith': (None, self.transform, 'text/plain'), |
There was a problem hiding this comment.
@ByronCinNZ does this work properly when one or more of these values are an empty string?
| </widget> | ||
| </item> | ||
| <item row="6" column="1"> | ||
| <widget class="QLineEdit" name="txtGeonetworkTransformWith"> |
There was a problem hiding this comment.
I am thinking of adding a dropdown here with all the available options for a more user-friendly experience. But this will do for now.
There was a problem hiding this comment.
I like the idea of a dropdown
| <item row="6" column="0"> | ||
| <widget class="QLabel" name="label_37"> | ||
| <property name="text"> | ||
| <string>Transform with</string> |
There was a problem hiding this comment.
I would prefer Transformation as a label here.
There was a problem hiding this comment.
agree - transformation is a better label
| <item row="5" column="0"> | ||
| <widget class="QLabel" name="label_35"> | ||
| <property name="text"> | ||
| <string>Assign to current Catalogue</string> |
There was a problem hiding this comment.
This is rather long imho. Need to clone your PR and see if the UI still looks okay...
Either way, I'd prefer a lowercase "c" for "catalogue". Also not sure about the British English spelling (consistency)...
There was a problem hiding this comment.
Also, since this is a boolean, we should perhaps turn this into a radio button or something 🤔
There was a problem hiding this comment.
British spelling seems to dominate but not always.
|
Agree with all your comments.
Yes, empty strings seem to work fine for “assignToCatalog” and "transformWith"
Yes, Group ID is better name. But an upgrade to this script would use the textual group name and translate it
UK or US spelling for “catalogue” - not sure consistency currently exist
So whichever you prefer
Yes to the radio button for “assignToCatalog” but a bit confused by the value that GN accepts for this - “on” for true and blank for false is what works for me and how it is used in the “Insert Metadata” function in GN (the api indicates true/false but I have had no success with the GN Swagger api for this)
Cheers,
Byron
… On 16/03/2022, at 12:22 AM, Sander Schaminee ***@***.***> wrote:
@GeoSander commented on this pull request.
In geocatbridge/servers/views/geonetwork.ui <#148 (comment)>:
> + </widget>
+ </item>
+ <item row="4" column="1">
+ <widget class="QLineEdit" name="txtGeonetworkGroup">
+ <property name="text">
+ <string/>
+ </property>
+ <property name="placeholderText">
+ <string>Group Id</string>
+ </property>
+ </widget>
+ </item>
+ <item row="5" column="0">
+ <widget class="QLabel" name="label_35">
+ <property name="text">
+ <string>Assign to current Catalogue</string>
Also, since this is a boolean, we should perhaps turn this into a radio button or something 🤔
—
Reply to this email directly, view it on GitHub <#148 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIJCPAR2OFIEJBROURYPGDVABXHXANCNFSM5QXHZWDQ>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.
|
|
Alright, thanks @ByronCinNZ.
Oof... that's nasty. Let me try out some things and find out what works best.
Not completely sure what you mean here, but Bridge for ArcMap actually queries all groups first on GN and populates a dropdown with available values (using the group name for display purposes and a numeric identifier as the actual value for each group). I could do the same for Bridge for QGIS of course, which I think is what you meant? |
|
For groups, yes a query for group name to populate a dropdown is what I was thinking |
|
I am on GN 3.12.2 |
GeoSander
force-pushed
the
master
branch
2 times, most recently
from
3e78ee0
to
2ac00f0
Compare
Extra parameters to allow flexibility on how GN handles uploaded metadata.