Invalidate cached project when project is remixed #1003
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes an issue which you can see if you follow these steps: 1. Login 2. Visit a project you do not own 3. Edit the code 4. Save the project (thus remixing it) 5. Visit the original project URL again 6. See your changes in the code when you should see the original version This was happening because after step 3 the auto-save functionality stored the project (with your code changes) in local storage against the original project identifier. When the project was saved, the remixed version was created in the database, but the cached version was left in local storage. When you re-visit the original project URL, the project cached in local storage was loaded, because the URL path matches the local storage key, i.e. the original project identifier. The fix is to invalidate the cached project after it is successfully remixed in a similar way to how the cached project is invalidated after it is successfully saved [1]. In this case, since we're *remixing*, the original project should always have an identifier, so we don't need to fallback to "project" like we do for saving. I did wonder whether we should only do this if the remixed project identifier (i.e. `action.payload.project.identifier`) is different from the original project identifier (i.e. `state.project.identifier`). However, I'm fairly confident this will always be true if we are remixing. Note that this came up when I was testing the changes in #992. [1]: https://github.com/RaspberryPiFoundation/editor-ui/blob/70ea01cefa38aa60d1d6d9fb4bc11188b6009049/src/redux/EditorSlice.js#L373 Fixes #1001.
floehopper
force-pushed
the
invalidate-local-storage-cached-project-after-remix
branch
from
f7444f8 to
3112e99
Compare
|
|
floehopper
added a commit
that referenced
this pull request
May 9, 2024
The items were all listed under the "Added" heading, but I'm pretty confident #976 & #991 were effectively internal fixes that had no impact on clients of the web component, and it feels like it's worth highlighting #1003 as a *change* to the existing behaviour rather than the addition of a new feature.
floehopper
added a commit
that referenced
this pull request
May 9, 2024
### Added - Support to enable embedding iframes in HTML projects from in-house domains (#985) - Dispatch event when project identifier changes, e.g. after project is remixed (#2830) - Add `load_remix_disabled` attribute to web component (#992) ### Changed - Invalidate cached project when project is remixed (#1003) ### Fixed - Unit tests for `pyodide` runner (#976) - Remove broken `format` script (#991)
floehopper
added a commit
that referenced
this pull request
May 24, 2024
The `editor-standalone` app will: * Listen for "editor-projectIdentifierChanged" custom events from the web component and will redirect to the remixed project's URL. The relevant web component behaviour was changed in #990. The relevant `editor-standalone` behaviour was changed in RaspberryPiFoundation/editor-standalone#63. * Set the web component `load_remix_disabled` attribute to `true` so that the original project will be loaded in preference to the remixed version (unlike the way projects-ui works). The relevant web component behaviour was changed in #992 & #1003. The relevant `editor-standalone` behaviour will be changed in RaspberryPiFoundation/editor-standalone#85. This spec checks that the web component provides the relevant behaviour for editor-standalone to use. I did attempt to add an example for the default behaviour (i.e. when `load_remix_disabled` is not set). However, without any way to easily reset the state of the projects in the editor-api database, it's hard to come up with a robust way to write such a test. Note that this PR changes both the editor app itself and the fake "app" wrapping the web component to use https://test-editor-api.raspberrypi.org instead of https://staging-editor-api.raspberrypi.org in CI. The former has `BYPASS_AUTH` set to `true` so the apps can make requests as if the user was logged in. The new e2e spec also relies on the `projects:create_all` rake task having been run - which we've only done manually for now, but needs automating.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue which you can see if you follow these steps:
This was happening because after step 3 the auto-save functionality stored the project (with your code changes) in local storage against the original project identifier. When the project was saved, the remixed version was created in the database, but the cached version was left in local storage. When you re-visit the original project URL, the project cached in local storage was loaded, because the URL path matches the local storage key, i.e. the original project identifier.
The fix is to invalidate the cached project after it is successfully remixed in a similar way to how the cached project is invalidated after it is successfully saved. In this case, since we're remixing, the original project should always have an identifier, so we don't need to fallback to "project" like we do for saving.
I did wonder whether we should only do this if the remixed project identifier (i.e.
action.payload.project.identifier) is different from the original project identifier (i.e.state.project.identifier). However, I'm fairly confident this will always be true if we are remixing.Note that this came up when I was testing the changes in #992.
Fixes #1001.