Data files removed from package when there isn't changePermission permission

[laurenwalker]

To reproduce:

• Create a data package with an object where the user only has 'read' permission. (This may happen with 'write' permission as well)

• Edit any metadata field in the package and click save

• The resource map, EML, and other data objects save successfully. However, the data object with 'read' only permission results in a 401, and it is removed from the package.

• When system metadata updates result in an error, we should still keep that object in the package.

• In addition, DataPackage.save() should only send a system metadata update call when there has been changes to the sys meta fields - filename, accesspolicy, or formatId

• We should work on the error messaging when there is a 401 error. Right now it shows the red error icon in that DataItemView, which is a little harsh. We could change this to a warning, instead. Or just work on the message text.

[robyngit]

Related to #1231 and #682

[robyngit]

When system metadata updates result in an error, we should still keep that object in the package.

In DataONEObject models, while the system metadata is being updated, the uploadStatus attribute is set to p for processing. If there is any issue updating the system metadata (including a 401 error) the model's uploadStatus is set to e for error. (At the same time, the sysMetaUploadStatus attribute is also set to e.)

When the DataPackage subsequently gets serialized, models that have uploadStatus set to e are excluded from the resource map XML, and thus removed from the package.

Commit e51839e fixes this by setting uploadStatus to w for warning when there is a 401 error updating the system metadata.

I still need to look into how this will impact other functions in MetacatUI that use the DataONEObject uploadStatus attribute, e.g. the AccessPolicyView.

I'm also wondering if we want to handle all types of system metadata update errors this way, or just 401 errors?

We should work on the error messaging when there is a 401 error. Right now it shows the red error icon in that DataItemView, which is a little harsh. We could change this to a warning, instead. Or just work on the message text.

This is how it looked before when there was a 401 error. Note also that the "describe" button was missing:

Here is how it looks with commit e51839e. It uses the w status to determine if a warning message is needed:

[laurenwalker]

@robyngit - Thanks, this looks great. I think we could definitely improve the wording of the warning message, but let's not let that block us from getting the improvements so far out the door.

Given this comment:

I still need to look into how this will impact other functions in MetacatUI that use the DataONEObject uploadStatus attribute, e.g. the AccessPolicyView.

Do you think your changes are ready for release? I see you committed them to the dev-2.14 branch and I am planning on releasing 2.14.0 ASAP.

[robyngit]

@laurenwalker These changes need some review to make sure they don't cause problems elsewhere in MetacatUI. I will do some testing and also improve the wording of the warning message. I've reverted the changes made to 2.14.0 and moved them to a feature branch.

[robyngit]

New warning message for sysMeta 401 errors:

[mbjones]

@robyngit I like the simplicity and the direction this is heading.

Nevertheless, the vast majority of MetacatUI users won't know what system metadata is, and this warning would be alarming without any indication of what to do about it. Can't we fix the issue so that these errors either can't occur/never occur, or are invisibly handled when they do occur?

It's not clear to me why we would be trying to modify system metadata on an object for which a only has read permission. I think that should be handled on the metacatUI side, and never even attempt that operation. We should be able to update a package without touching the system metadata for the data objects in that package.

If the user has write permission but not changePermission permission on a file, then they should be able to make changes to the object (and therefore the system metadata) as long as they don't touch the access rules. So, if the metacatUI makes sure they only change the allowable parts of system metadata, wouldn't they be all set?

Finally, if an attempt to modify system metadata fails, I don't think displaying an error to the user is helpful without providing a means to do something about it, or without doing something about it ourselves. So, if we get an error, I'd rather see something like "You don't have permission to change access rules, so we updated the object but did not change the access rules." I think references to "system metadata" probably are not warranted in the UI.

[robyngit]

Can't we fix the issue so that these errors either can't occur/never occur, or are invisibly handled when they do occur?

Previously, system metadata updates were attempted for every object when the user saved the data package, regardless of whether they were required. That has now been fixed, and system metadata updates are only sent when there are changes.

MetacatUI does still allow the user to try and re-name a file for which they only have read permission, and that would lead to this warning messaging appearing when they save. We could disable renaming in this case, similar to how we disabled replacing objects without sufficent permissions in #1393.

Finally, if an attempt to modify system metadata fails, I don't think displaying an error to the user is helpful without providing a means to do something about it, or without doing something about it ourselves.

Good point, I'll work on the messaging.

[laurenwalker]

We could disable renaming in this case, similar to how we disabled replacing objects without sufficent permissions in #1393.

I think this is the way to go. I still like showing the warning and a message as a backup in case Metacat does respond with a 401 anyway.

[robyngit]

I think this is the way to go. I still like showing the warning and a message as a backup in case Metacat does respond with a 401 anyway.

Done in the last commit. I also added tooltips to match those added in #1393.

When you can rename (with text cursor):

When you can't rename (with not-allowed cursor):

The warning will still appear in case there is a 401 error, but with improved messaging similar to Matt's example

[laurenwalker]

Looks great, Robyn!

[laurenwalker]

While reviewing Robyn's feature branch, I have discovered that this issue exists for data objects that you have write permission on as well. If you only rename a file in MetacatUI, a system metadata update is triggered on Submit, and Metacat returns a 401. Even though you have not made any changes to the access policy.

The error causes MetacatUI to remove the data object from the package (This behavior should be improved as well...we could have kept the original version in the package with no issues)

I created a Metacat issue for this: NCEAS/metacat#1475 because I think the way we handle system metadata updates should change. But in the meantime, we should start disabling renaming files to those without changePermission authorization, too.

A next step to take while we wait for Metacat to change, is to allow renaming, but perform an update() on the object to get around the updateSystemMetadata() 401 error. This is a bit overkill - creating a new version of an object just to rename it - but it allows the user to get the job done.

[laurenwalker]

After further review of this branch: This solution wasn't really working for me. I could see the differences in the messaging, but the functionality of handling a 401 on updateSystemMetadata() was not improved. I found it more confusing to add another uploadStatus when we already distinguish sys meta fails with a sysMetaUploadStatus.

I removed the "w" uploadStatus and changed the functionality of the DataPackage.save() so that it keeps objects in the package when sysMetaUploadStatus == "e". The DataItemView can still display a specific message when the sys metaupdate fails.