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

Cannot edit childCO in a COG #5481

Closed
emenslin opened this issue Dec 17, 2024 · 2 comments · Fixed by #5505
Closed

Cannot edit childCO in a COG #5481

emenslin opened this issue Dec 17, 2024 · 2 comments · Fixed by #5505
Assignees
Labels
1 - Bug Incorrect behavior of the product 2 - Forms Issues that are related to the form system geospecify regression This is behavior that once worked that has broken. Must be resolved before the next release.
Milestone

Comments

@emenslin
Copy link
Collaborator

Describe the bug
If you edit a CO through the children subview in a COG the save button is not enabled and you can't save, if you make a change to the COG form to enable the save button it throws an out of date error. This might already be an existing issue but I could not find it.

To Reproduce
Steps to reproduce the behavior:

  1. Go to COG
  2. Add ChildCO
  3. Expand CO
  4. Edit a field
  5. See save is not enabled
  6. Change something on the COG form
  7. Save
  8. See error

Expected behavior
You should be able to edit a CO in the COG form.

Screenshots

chrome_aK4ADOhQWB.mp4

Crash Report

Specify 7 Crash Report - 2024-12-17T16_47_13.357Z.txt

Please fill out the following information manually:

@emenslin emenslin added 1 - Bug Incorrect behavior of the product 2 - Forms Issues that are related to the form system geospecify labels Dec 17, 2024
@grantfitzsimmons grantfitzsimmons added the regression This is behavior that once worked that has broken. Must be resolved before the next release. label Dec 17, 2024
@grantfitzsimmons grantfitzsimmons added this to the 7.9.10 milestone Dec 17, 2024
@grantfitzsimmons
Copy link
Member

Seems this was broken at some point in Geo development. This was reported as a usability concern by André and Soraya from the Geology Swiss steering committee

@melton-jason
Copy link
Contributor

melton-jason commented Dec 18, 2024

This is ultimately because some part of one-to-one relationships are not being properly supported.

More specifically in this case: the change/saverequired event in the CollectionObject is not being propagated up to the CollectionObjectGroupJoin and CollectionObjectGroup resources.

This behavior is handled via the the event handlers in resourceApi.ts:

function eventHandlerForToOne(related, field) {
return function (event) {
const args = _.toArray(arguments);
switch (event) {
case 'saverequired': {
this.handleChanged();
return;
}
case 'change:id': {
this.set(field.name, related.url());
return;
}
case 'changing': {
this.trigger.apply(this, args);
return;
}
}
// Pass change:field events up the tree, updating fields with dot notation
const match = /^r?(change):(.*)$/.exec(event);
if (match) {
args[0] = `r${match[1]}:${field.name.toLowerCase()}.${match[2]}`;
this.trigger.apply(this, args);
}
};
}
function eventHandlerForToMany(related, field) {
return function (event) {
const args = _.toArray(arguments);
switch (event) {
case 'changing': {
this.trigger.apply(this, args);
break;
}
case 'saverequired': {
this.handleChanged();
break;
}
case 'change':
case 'add':
case 'remove': {
// Annotate add and remove events with the field in which they occurred
args[0] = `${event}:${field.name.toLowerCase()}`;
this.trigger.apply(this, args);
Reflect.apply(this.trigger, this, ['change', this, related]);
break;
}
}
};
}

this.on('change', function () {
if (!this._fetch && !this._save) {
this.handleChanged();
this.trigger('saverequired');
}
});

For which the saverequired event is caught by the useIsModified hook:

export function useIsModified(
resource: SpecifyResource<AnySchema> | undefined,
// Whether a new resource that hasn't been modified is treated as not modified
ignoreBrandNew = true
): boolean {
const [saveRequired, handleNeedsSaving, handleSaved] = useBooleanState(
resource?.needsSaved === true && (!resource?.isNew() || !ignoreBrandNew)
);
// Recompute default value when resource changes
React.useEffect(
() =>
resource?.needsSaved === true && (!resource?.isNew() || !ignoreBrandNew)
? handleNeedsSaving()
: handleSaved(),
[resource, ignoreBrandNew, handleNeedsSaving, handleSaved]
);
// Listen for "saveRequired"
React.useEffect(
() =>
typeof resource === 'object'
? resourceOn(resource, 'saveRequired', tap(handleNeedsSaving), false)
: undefined,
[resource, handleNeedsSaving]
);
// Listen for "saved"
React.useEffect(
() =>
typeof resource === 'object'
? resourceOn(resource, 'saved', handleSaved, false)
: undefined,
[resource, handleSaved]
);
return saveRequired;
}

I imagine most of the work for this fix would be investigating why the event(s) are not being propagated: the actual change would likely be a very small one

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 - Forms Issues that are related to the form system geospecify regression This is behavior that once worked that has broken. Must be resolved before the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants