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

Inconsistent NewVersion button behavior on some records #2197

Closed
Samk13 opened this issue Apr 18, 2023 · 9 comments · Fixed by inveniosoftware/invenio-drafts-resources#232
Closed
Assignees
Labels
bug Something isn't working

Comments

@Samk13
Copy link
Member

Samk13 commented Apr 18, 2023

Package version (if known): 12 beta

Describe the bug

On some records, when you click on edit and then new version, you end up in a state where you can't keep going, the GIF video will explain it:

test-version-on-second-community

Steps to Reproduce

  1. Go to 'record'
  2. Click on 'edit'
  3. Click on 'create new version'
  4. See that nothing happen

Expected behavior

See the import file button, and I am able to edit files.

Additional context

It's hard to replicate it on each record, and I can't find what is causing the problem, I created another record from scratch and it works as expected.

@Samk13 Samk13 added the bug Something isn't working label Apr 18, 2023
@tmorrell
Copy link
Contributor

We've seen this behavior sporadically in our v9 production instance. It's also intermittent-you can sometimes come back to a record where "new version" didn't work after a few hours and it will work again. I haven't found any pattern to when it happens.

@tmorrell
Copy link
Contributor

tmorrell commented Apr 18, 2023

I coincidentally had this crop up with a user in production today, so I spent more time debugging. The UI behavior occurs when a new version draft fails to be created. There aren't any error messages, and the post to /versions returns the original record metadata so the application continues as normal. The deposit form is happy to load the /versions response, but the metadata is the same as the original record so the deposit form loads something that looks a draft record edit screen. However, since no draft was created doing anything in that screen will result in unhappiness.

I found a workaround by creating the new version programmatically with:

from invenio_utilities_tuw.utils import get_identity_for_user, get_record_service

u = get_identity_for_user(2)
service = get_record_service()

version = service.new_version(id_=pid, identity=u)

My assessment is that /versions needs to be stricter in really returning a new version (or bubbling up an error message), but I don't know what the underlying error is.

@Samk13 Samk13 changed the title Inconsistent New Version Button Behavior on some records Inconsistent NewVersion button behavior on some records Apr 19, 2023
@tmorrell
Copy link
Contributor

This came up again, and I got farther in debugging. My solution above doesn't work universally.

What I observed today was a record where 'next_draft_id' got set to the identifier of the record.

Output of record.versions: VersionsManager (parent_id: c1e422cb-dff8-47eb-9482-3fc9f9b5d86c, index: 1, latest_id: 882c7be5-9826-4bb4-b4be-53dfc13eaa25, latest_index: 1, next_draft_id: 882c7be5-9826-4bb4-b4be-53dfc13eaa25)

This resulted in the new_version function call not working, since it just returned the parent record https://github.com/inveniosoftware/invenio-drafts-resources/blob/02c33f144e0b1fbf6796a5c1671397a39fdb0208/invenio_drafts_resources/services/records/service.py#L370

I was able to clear this with the following script:

record = service.record_cls.pid.resolve(pid)
    print(record.versions)
    record.versions.clear_next()
    record.commit()
    db.session.commit()
    service.indexer.index(record)

Is there a scenario where latest_id and next_draft_id should be the same? Maybe if there is an additional check that next_draft_id and last_id are different added at https://github.com/inveniosoftware/invenio-drafts-resources/blob/02c33f144e0b1fbf6796a5c1671397a39fdb0208/invenio_drafts_resources/services/records/service.py#L370 the problem would be solved?

I'm still not sure how next_draft_id ended up getting set in the first place.

@fenekku
Copy link
Contributor

fenekku commented May 25, 2023

This cropped up for us too on v11 .

What we noticed is that clicking on New Version from a record landing page would lead to a page with previous version files already loaded. Clicking on new version again would seemingly do "nothing".

After some quick look, what seems to happen is that the page is reloaded before the POST has completed:

interrupted_pos

so the new version is not made and the previous version is reloaded (with previous files obviously). Seems like a frontend race that could explain why it shows up some of the time. So basically, we occasionally can't create a new version right now.

@kpsherva kpsherva assigned TLGINO and unassigned TLGINO Jun 7, 2023
@fenekku
Copy link
Contributor

fenekku commented Jun 26, 2023

We had this sitting in our backlog and it's causing issues to our users, so I might as well give it a shot.

@zguillen
Copy link

Hello all:
I have some bummer news to add to this issue. We are seeing this same issue on our prod instance of RDM. I applied the fix from the referenced PR to our v9.1 RDM instance and it did not change this behavior for us. I simply copied the one line “if not record.is_published:” over and didn’t merge the PR or anything.

Another larger bummer is that not only do we get this behavior originally reported but an error in the UI as well for this case:

  1. Click Edit on a published record
  2. Click New version (UI renders edit draft of published instead of new version draft)
  3. Click discard changes.
  4. Go back to published record’s landing page and click new version

UI will now show this error message:
image

Exception printed in the log is, “sqlalchemy.exc.NoResultFound: No row was found when one was required” triggered from this line “next_draft = self.draft_cls.get_record(” of invenio_drafts_resources/services/records/service.py new_version.

Once a record gets into this state the only way I’ve been able to recover from it is to run the script @tmorrell posted above (a BIG thank you!) #2197 (comment)

Is there any advice you have for me on this? I’m pretty stumped still ☹

@zzacharo
Copy link
Member

Hello, @zguillen! Thanks a lot for reporting this issue and for allowing us to dig more into it! We investigated the issue and the fix from @fenekku is actually working! Let me elaborate a bit more on that:

In InvenioRDM, when a record is published, the corresponding draft is soft-deleted i.e. stays in the database but in a "deleted" state. Moreover, we have a task that runs by default every hour and cleans up all the soft deleted records i.e these drafts are no longer in the database! Now, if you try to edit a record whose draft was cleaned up then if you don't have the fix from @fenekku you end up in the situation you experienced. That is actually confirmed working in the latest v12 beta releases. The only potential problem you could have is if in your instance you have changed the time of the cleanup_drafts cronjob to be 1 minute! In that case, you would need this fix inveniosoftware/invenio-drafts-resources#244 !

Now, for your fix: Is there any code link we can check to ensure the fix is applied correctly? Can you replicate this bug in your instance consistently e.g if you follow this workflow for published records with 1 hour difference between edits (the 1 hour is the time for the cleanup task to have purged all soft deleted drafts). Also if it's more convenient we can continue debugging this in #rdm-help channel in discord!

@zguillen
Copy link

zguillen commented Aug 25, 2023

Yay, @zzacharo! this PR is fixing the bug for me now. I don't know what was going on when I applied the fix originally but still seeing the error...I must have been testing with records already in a bad state?

My steps to reproduce the incorrect behavior and error were:

  1. create and publish a record
  2. ->important<- wait an hour to be sure the cleanup_drafts ran
  3. click edit button on the published record
  4. go back and click the new version button
  5. --->this is where the UI will show the edit draft page instead of new version draft
  6. click the discard draft button
  7. --->this is where the UI will render the not found error

after applying the fix from this PR I no longer get the behavior in step 5 nor the error in step 7. Thank you so much!

@zzacharo
Copy link
Member

Hey @zguillen ! That's great! Most likely it was because, as you said, you were testing your fix in already broken records but having also the fix from the inveniosoftware/invenio-drafts-resources#244 will double save any indexing problem you might experience if you change the interval of the cleanup_drafts task!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Released | Done 🚀
Development

Successfully merging a pull request may close this issue.

6 participants