Improve CollectionRelationship uploads in the WorkBench

[melton-jason]

Fixes #3089

---

This pull request includes:

• Autogenerated default forms for CollectionRelationship and CollectionRelType

  • I believe Specify 6 contains a view definition for CollectionRelType. Which of course does not work in Specify 7. So primarily ensure that the default form for CollectionRelationship is OK.

• Apply the proper schema overrides for CollectionRelationship and CollectionRelType.

  • CollectionRelType was made requried for CollectionRelationship
  • name was made required for CollectionRelType

• Makes CollectionRelType.name a frontend-only picklist with name as its field

  • CollectionRelType.name is how the collections are supposed to be determined during the upload
  • Because this is such an important field, this change will ensure that only existing names are used

• Adds the ability to override which collection a column will upload into

  • To accomplish this, go into the uploadplan (A video is attached detailing this process)

  • Find the correct relationship/table/column you want to override

  • As a key/value pair for an uploadTable, add "overrideScope" : {"collection" : COLLECTION_ID}, where COLLECTION_ID is your desired collection to upload into

  • NOTE: This override should work for any table being uploaded through the WorkBench, not just CollectionRelationships

---

To View Upload Plan

wb_show_plan.mp4

* a video showing how to view the [DEV] Show Plan button

---

Collection Override Example

* an example of a collection override being applied to the rightside of a CollectionRelationship, with the additions marked by an orange box

[melton-jason]

The only remaining thing to accomplish with this Pull Request is to have the backend automatically override the rightside collection when uploading a CollectionRelationship.

The intended behavior can also currently be accomplished by adding a manual collection override for the rightside field in the WorkBench upload plan
(see my steps outlined in the intial PR comment under the Adds the ability to override which collection a column will upload into section for instructions to test this).

[maxpatiiuk]

Looks great!

So if I understood correctly, front-end doesn't need to fill out overrideScope, as back-end will do it automatically for CollectionRelatinships, but if it would be necessary, someone can manually edit the upload plan?

Also, if I enter overrideScope, then edit upload plan in the mapper and save, the overrideScope won't be preserved? Not an issue, but something to be aware of

[melton-jason]

So if I understood correctly, front-end doesn't need to fill out overrideScope, as back-end will do it automatically for CollectionRelatinships, but if it would be necessary, someone can manually edit the upload plan?

Yes, to stay as compatible as possible with past workbenches, I made overrideScope optional in the upload plan. We will need to add it automatically on the backend for things like CollectionRelationships.

And yes, if needed someone can manually edit the upload plan and tell the backend which collection to validate and upload into. With this change, some use cases might be uploading Collection Objects in multiple collections with one WorkBench dataset, or upload/validate PickLists for a separate collection, etc...
I tried to make it as easy to scale as possible, so in the future we can provide customizability for every scope.
If this is a popular idea, we could provide some UI in the WorkBench that would just add the overrideScope to the upload plan.

Also, if I enter overrideScope, then edit upload plan in the mapper and save, the overrideScope won't be preserved? Not an issue, but something to be aware of

The upload plan is stored in the database. Editing and then saving the upload plan saves it to the database, so overrrideScope will be preserved.

[maxpatiiuk]

I like how you implemented it, but I don't think there would be that much need for this beyond CollectionRelationships - most of the time, if users want to upload into a different collection, they would just switch to that collection

[maxpatiiuk]

The code for deferring scope looks tricky. Sorry can't comprehend it right now as I don't know well what's going on here.

If there is a way to simplify it then do it, otherwise document what the DeferedUploadPlan concept means and how it interacts with UploadPlan. Also, let's keep up the tradition of workbench have very good test coverage and add test cases for the newly added code - that will also help explain to others what's the expected behavior

[melton-jason]

This is ready to be tested. I will fix any failing tests and add comments/documentation either tonight or tomorrow, but all of the logic is in place.

When testing, please make sure that right-side collection objects get uploaded/matched correctly in the rightSideCollection of the collection rel type. (Also, test to ensure that it validates and uploads with the correct collection's Catalog Number formatting).

Please also do brief testing of common workbench uploads/functions, as well as for recordsets.

[grantfitzsimmons]

I am so impressed just reading your summary and code. Excited to try this and so happy to see this!!!

[melton-jason]

I was considering my changes and realized there is a bug right now with uploading CollectionRelationships with more than one CollectionRelTypes that have differing rightSideCollections.

Take this WorkBench dataset for example:

If the HostFish CollectionRelType has a separate rightSideCollection than TestRel, it will be ignored and upload that row's rightSide CollectionObject 111 into TestRel's rightSideCollection.

I am working on a fix, but expect more complicated WorkBench code!

You can continue to test this as normal, I am simply pointing out I am aware of this bug to prevent redundancy

[melton-jason]

I was considering my changes and realized there is a bug right now with uploading CollectionRelationships with more than one CollectionRelTypes that have differing rightSideCollections.

Take this WorkBench dataset for example:

If the HostFish CollectionRelType has a separate rightSideCollection than TestRel, it will be ignored and upload that row's rightSide CollectionObject 111 into TestRel's rightSideCollection.

I am working on a fix, but expect more complicated WorkBench code!

You can continue to test this as normal, I am simply pointing out I am aware of this bug to prevent redundancy

This should now be fixed.

Assuming no more bugs present themselves, all that is remaining is to add documentation in the code and update/write tests.

[realVinayak]

I'm getting this error when I try to save this mapping.

[grantfitzsimmons]

I'm getting this error when I try to save this mapping.

Can you request changes on the PR?

[realVinayak]

Saving mapping throws an error -

[maxpatiiuk]

NOTE: This override should work for any table being uploaded through the WorkBench, not just CollectionRelationships

Does it check for permissions when uploading into a different collection?

[melton-jason]

I've (finally) resolved all of the failing tests.
Can this briefly be tested again? If everything is still good, then this can be merged! 🥳

[grantfitzsimmons]

@chanulee1 Can you test this today?

[melton-jason]

Can't believe I didn't catch this before. Can we have Collection Rel Type be required, or put a default value in? It throws this error otherwise - it should just highlight it red.

Also have it highlight the empty catalog number (or any other required cells) after validation if they are empty. I was trying to validate it without putting in a catalog number.

Collection Rel Type and the Left/Ride side Collection Objects are required, but only in the frontend (as schema overrides). Thus they are not actually required on a database level.

The cell highlighting is done by checking if the SpLocaleContainerItem associated with each column of the upload is required in the database.

specify7/specifyweb/workbench/upload/parsing.py

Lines 60 to 71 in e96288c

	required_by_schema = colopts.schemaitem and colopts.schemaitem.isrequired
	result: Union[ParseResult, ParseFailure]
	was_blank = value_in.strip() == ""
	if was_blank:
	if colopts.default is None:
	missing_required = (
	"field is required by upload plan mapping" if not colopts.nullAllowed else
	"field is required by schema config" if required_by_schema else
	None
	)
	result = ParseResult({fieldname: None}, {}, None, colopts.column, missing_required)

Preferably, we run a Django migration which properly marks the correct fields as required in the database. However, to be thorough, we would additionally need to modify the Specify6 datamodel xml (The django fields are built from the xml). We may even

Alternatively, we can make schema overrides for the backend. However, I do not like this solution because it fragments the schema even further. There would be front-end overrides, back-end overrides, and then what the value is in the database.

[melton-jason]

I have talked with @grantfitzsimmons and we have reached a conclusion about how to approach this.
The solution will be painful regardless of what we do.

If we solve the root of the problem, alongside a django migration, we would need to modify Specify's 6 specify_datamodel.xml, as Specify 7 relies on it to build the datamodel. However, the specify_datamodel.xml is autogenerated by Specify 6 when it is compiled, so this would required a Specify 6 update.
(Preferably we move away from relying on the outdated xml. Having a Specify 7-only representation of the datamodel would also fix #2813)

Alternatively, adding schema overrides to the backend would needlessly make debugging in the future more difficult, as the schema would be split into 3 pieces (the frontend representation, backend, and database) and cost more development time in the future. The solution would be 'hacky', and not long term.

For now, unless any further ideas are brought forward, it may be wise to leave this as is for now and provide documentation explaining the behavior. A proper solution-regardless of the way we go-will warrant further discussion.

[grantfitzsimmons]

@realVinayak and @chanulee1 – can you test this when you get a chance?

[chanulee1]

Looks good. Right side collection object validated using right collection's catalog number format. Right side collection object is created when catalog number does not exist in right side collection. Collection Relationship uploads seem to be working well.

[realVinayak]

See #4929 for an alternate implementation. The main difference is that I consider scoping completely different than binding. Instead of scoping upload plan "constantly" (which then required defer logic per-row), I scope it for every row (so defer isn't needed -- just scope). Simplifies things this way.

For performance, observe that upload plan only needs to be rescoped if there are collection relationships (or any other "defer" fields). So, it checks for them when doing scoping. If not found, it is safe to cache it. Hence, the little caching mechanism in that implementation. That is, the upload plan gets scoped just once (for the first row, the next rows reuse it as long as no defer fields are found), which is the common case (most wb uses don't involve collection relationships).