- Issue created by @tedbow
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Couple of questions on the current progress:
* I don't think we should pass the data as an array, can we not just keep it as a string and save the performance overhead of serialize\unserialize being called (and particularly the memory impact). Would likely need our own schema for the tempstore to bypass the serializing of the data column
* Should we also be considering langcode here in the identifiers? - 🇺🇸United States tedbow Ithaca, NY, USA
I pushed up a start that just replaces the current trait with a service. It doesn't change the basic functionality.
It does not add any of the extra metadata, timestamp, author, etc to the auto-save yet.
One thing we will have to consider eventually is that by 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active we will be dealing with new entities, not just existing ones. We will also have a "publish all" button that takes all of the auto-saved work and saves them to real entity revisions. For new entities that are created in XB that means we can not rely on entity type/entity id for the auto-save key because they will not exist as entities yet. Maybe
\Drupal\experience_builder\AutoSaveManager::save
should return back the auto-save key(a uuid?) so that client can request the auto-save state back later when it there is no entity involved. - 🇺🇸United States tedbow Ithaca, NY, USA
Also right now the e2e still don't test auto-save. There is hack in the test to get around it
we should probably commit 📌 Adapt E2E tests to work with auto-save Active , this issue. But it doesn't block it now
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Rebased the MR on top of 📌 Create an endpoint to publish all auto-saved entities Active
Adding PP-1 to the title to reflect that should go in first and then this should be rebased
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
https://git.drupalcode.org/issue/experience_builder-3489994/-/merge_requ... shows the changes on top of 📌 Create an endpoint to publish all auto-saved entities Active
The merge request seen above is including that branch
- 🇺🇸United States tedbow Ithaca, NY, USA
Changes since my last review look good. I think this is basically ready. Will review again after 📌 Create an endpoint to publish all auto-saved entities Active is and we can remove those changes
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
This is ready for review now, there's one question on the MR about whether we should ship with an image style for the avatars or not
- 🇺🇸United States tedbow Ithaca, NY, USA
I talked to @effulgentsia about this issue and 📌 Create an endpoint to publish all auto-saved entities Active which together will allow the UI to create "Publish all" button
I think we missed something between these 2 issues
Here is the UI proposed workflow
- User clicks "Ready to publish"
- User sees a list of all entities in the auto-save state that are ready to be published and the dates they were last changed
- User clicks "Confirms"
- if auto-save states matches what the user just saw the all the changes will be published. This means there can't be any new changes since the user saw the list. No new entities, no entities missing, no new changes to the entities
- if there are changes since the user saw the list they will be prompted to confirm after seeing the new list.
So as it is now
ApiPublishAllController
doesn't take any input from the client but we probably need to change that where the client sends back some info that it received fromApiAutoSaveController
. Otherwise the user could see a list of changes to be published wait an hour while other changes are made, without them being aware, click "publish all" and then they have published things they are not away of. - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Added https://www.drupal.org/project/experience_builder/issues/3491459 ✨ [PP-1] Create api slice for auto-save data (pending changes) and publishing all data Postponed
And in doing so it makes me feel the best naming for the controller here would be something involving 'pending changes'
Remaining tasks:
* Add conflict IDs and updated data to body of the 409
* update openapi.yml to reflect possible 409 and presence of the data_hash key
* Add image style
* Rename controller and consider consolidation with the publish all one
* Rebase off 0.x now that 408 is in