Remove default scope excluding scratch projects #558
Conversation
raspberrypiherokubot
temporarily deployed
to
editor-api-p-remove-def-aamviv
Many thanks for your reviewing. I've responded to your questions inline. Let me know if you need any further clarification or want me to change anything before merging. Also can you confirm that you now have the measures we discussed in place in editor-standalone & editor-ui to handle the case where someone tries to load a scratch project into the Code Editor? Just wanted to triple-check! |
## Status Related to RaspberryPiFoundation/digital-editor-issues#648 and #558 ## What's changed? - Added project type to the project context route to support redirecting away from Scratch projects on the frontend - Removed project finished data from the lessons index (which appears not to have been used and was throwing errors)
Thanks James @floehopper, we've just merged the frontend changes and a small backend tweak to support this, so Scratch projects shouldn't get shown in the editor and editor site should redirect people if they hit a direct link to a scratch project. Once we've got this PR in we can check things out on staging a bit more easily. Only remaining thought is the comment I've replied to above, otherwise happy with this. |
This reverts commit 6266ba8.
This reverts commit 971e52f.
This reverts commit d635cef.
This effectively reverts this commit [1] from #513, but keeps some of the other changes [2,3] from #513. Note that at this point the examples in `spec/requests/projects/destroy_spec.rb` and `spec/features/project/updating_a_project_spec.rb` which have been added since #513 can't possible work with the default scope still in place, but without the scratch-only option. I've also updated `lib/tasks/projects.rake` which has also changed since #513 to cope with the removal of the `only_scratch` scope. I plan to remove the default scope in a subsequent commit which will effectively revert the only other commi [4] from #513 at which point those examples should pass again. [1]: 56022c6 [2]: 64d9e89 [3]: 920fd62 [4]: caf1a57
|
Rebasing on |
floehopper
force-pushed
the
remove-default-scope-excluding-scratch-projects
branch
from
85a9c10 to
6d86cc8
Compare
This removes the default scope on
Projectthat hid scratch projects by default which was originally added in #513 to support work on the Experience CS project while leaving Code Editor, Code Editor for Education, etc, unaffected.The Code Editor team are going to add protection to
editor-standaloneandeditor-uiso that they show an error message and/or redirect the user if they are unintentionally used to load a scratch project. Once this protection is in place, we'll be able to apply the changes in this pull request to remove the default scope so that scratch projects are no longer hidden. This is necessary to support ongoing integration between Experience CS and Code Editor for Education.As we agreed, I haven't made any changes to the GraphQL endpoint. In any case, I think any changes to filter on
project_typewould need to be made on the client side.I decided not to fully revert all the changes, because I think extracting the project type string literals into
Project::Types::PYTHON,Project::Types::HTML&Project::Types::SCRATCHis still a useful change. And because of that and some later changes, I wasn't able to simply revert a couple of the commits, so I constructed those commits manually.I decided to remove the
scratch_onlyscoping mechanism as well as the default scope, because I don't think we really need it inexperience-cs- I think we can probably replace it with something similar to what the Code Editor team are going to do, i.e. display an error and/or redirect if the app is unintentionally used to load a non-scratch project. Having said that, there's no need to wait for us to make that change before merging this pull request.