Skip to content

Find unapplied changes and ensure we get updates from the backend about those.#3771

Merged
marcellamaki merged 2 commits intolearningequality:hotfixesfrom
rtibbles:not_applicable
Oct 25, 2022
Merged

Find unapplied changes and ensure we get updates from the backend about those.#3771
marcellamaki merged 2 commits intolearningequality:hotfixesfrom
rtibbles:not_applicable

Conversation

@rtibbles
Copy link
Copy Markdown
Member

Summary

Description of the change(s) you made

  • Ensures that tasks that are long running in their application and emit their own changes during their running (like copying, publishing, syncing etc.) get properly finalized and marked as succeeded
  • Does this by looking for changes that are marked as synced but with no errors nor disallowed, which means they have been sent to the backend and are awaiting application
  • If this value is less than the currently registered max rev for the channel, it sends that instead.

Manual verification steps performed

  1. Do a long running copy.
  2. Ensure that the sync returns allowed, then that it returns the in progress tracking, and then that the change event is included in success (and that the channel rev sent to the backend is the server_rev for the copy change until it has succeeded

References

Fixes #3753

Copy link
Copy Markdown
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started by confirming that I could reproduce the original bug locally - which I could. After running the devserver with these changes, I cannot reproduce it -- so looks good to me 🎉 Did a code walkthrough with @rtibbles and I think the changes make sense, and it does not seem to be breaking anything else with my initial QA pass. Will merge, and @radinamatic and @pcenov can confirm that what I see locally is also reflected in hotfixes 🤞

@pcenov
Copy link
Copy Markdown
Member

pcenov commented Oct 26, 2022

Hi @rtibbles, now the originally reported issue is fixed as the contents of the imported folders are displayed.
The newly introduced issue however is that the Publish button remains disabled after both importing or uploading a resource. I tried refreshing the page or going back to the channels page but this results in a brief activation of the button and then it gets disabled again:

2022-10-26_16-49-47.mp4

And a screenshot with the console errors:
2022-10-26_17-24-18

@rtibbles rtibbles deleted the not_applicable branch October 26, 2022 16:11
@rtibbles
Copy link
Copy Markdown
Member Author

So, I think the specific issue you are seeing here is a result of residual changes left in the local database from earlier testing. I have replicated with the same residual data. Signing out and signing back in before doing any more follow up testing will be important!

It did however make me realize that there is a bug in this fix which I will submit a follow up PR for.

@pcenov
Copy link
Copy Markdown
Member

pcenov commented Oct 27, 2022

The issue with the disabled Publish button was tested and observed with different users in several browsers in incognito mode with no preexisting database from earlier testing. The errors in the console however might be from residual changes left in the local database in one of the browsers...

@rtibbles
Copy link
Copy Markdown
Member Author

OK, that makes sense, thanks!

@bjester bjester mentioned this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants