inspirehep / inspire-next

The INSPIRE repo.
https://inspirehep.net
GNU General Public License v3.0
59 stars 69 forks source link

literature submission: bug in editor when saving #3045

Open annetteholtkamp opened 6 years ago

annetteholtkamp commented 6 years ago

When I make changes via the editor of a submitted record the "save" button does not lead back to the submission but produces https://labs.inspirehep.net/editor/literature/undefined

david-caro commented 6 years ago

@harunurhan The editor when saving an edit from the workflows is first sending a PUT to the record, and then a POST to the resolve endpoint.

When the edit is from a workflow (click the edit button on a non-merge workflow) it should not actually do that last post, as the record is not really being resolved (there's still the decision to make it core/non-core).

Currently that last post, messes up the workflow and leaves it in a unexpected state.

Do you know if that's something we can easily do?

harunurhan commented 6 years ago

Reason that is routing to /literature/undefined is that workflow object doesn't have workflowObject['metadata']['control_number'] is actually undefined which shouldn't be. @david-caro, @jacquerie or @ammirate might know why.

For what you said @david-caro Resolve endpoint is called no matter what when workflow without conflicts (['_extra_data']['conflicts'] is not defined or empty list) is saved successfully. What I can do is here save initial is state of conflicts when I load the workflow object and later do the call if there were some conflicts and now there aren't. But I must warn that frontend bloating with logic that should not be there. From the beginning this resolve should have been done within or after PUT without requiring more requests.

michamos commented 6 years ago

it's normal that a new record in the holding pen doesn't have a control number yet. Those get assigned when creating the final record.

david-caro commented 6 years ago

@harunurhan maybe the problem is that we are using the editor for two different things, editing records, and resolving conflicts...

What if instead of doing the POST and PUT, we pass a callback, and handle that there? For the conflict resolving, that would be the resolve endpoint, and for normal edits the rest endpoint :/ Does that sound reasonable?

harunurhan commented 6 years ago

@michamos ok then I will need resolve endpoint to return the control_number since it's syncly saving/creating the final record.

@david-caro I don't get it, what would resolve one will get and return?

david-caro commented 6 years ago

ok then I will need resolve endpoint to return the control_number since it's syncly saving/creating the final record.

That can't be, as at that point of the workflow, you don't have a record, it's not saving a record. But, what you need is not the control_number but the workflow id (that's what you put in the editor endpoint at first too). Hmm... we could try to return there that instead, what is the exact data that it expects? a dict like:

{
    "control_number": 1234,
}

?

The idea of passing callbacks will only work well if we pass the same to all of them, so the backend would send a callback url to the editor, and when saving, the editor will use only that callback. For it to be useful the editor should be agnostic to the callback, and all should have the same signature and return a similar object response that the editor can understand (I'm not sure how many changes are needed right now to make the signatures the same on all those endpoints).