Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
When nodes are part of a book outline from Drupal core's book module, then that outline information is lost after sync.
Remaining tasks
- Find out, how to include the book settings into the json api serialization
- Add the node uuids of all referenced parent nodes and books and manage them for sharing just like entity references
- On the client side, handle the parent nodes and book references and store the book outline information
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2022-03-02_16-03.png | 77.42 KB | jurgenhaas |
Issue fork entity_share-3266530
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3266530-rebased
changes, plain diff MR !68 - 3266530-add-support-for changes, plain diff MR !46
1 hidden branch
Comments
Comment #3
jurgenhaasI've had extensive discussions with maintainers and users of the JSON:API, and it turns out, that the 2 core components JSON:API and books don't work together. Also, there are no plans to change that any time soon. Therefore, I suggest solving this requirement in separate submodules here in entity share:
entity_share_book_server: this should be enabled on the server together withentity_share_server- it will add extra book related data to the data being sent back, if a node belongs to a book.entity_share_book_client: this should be enabled on the client together withentity_share_client- it will read the extra book, if available, and stores the extra information in the client's database.The server component is already available and is done with a response event subscriber. The code is in MR !46, and I've tested it with larger books, there is no measurable performance impact. The extra output keeps the JSON:API data compatible, and it looks like this:
In addition, I will have to develop the client part of this too, but I have a question before I can get started: we receive the UUIDs of all referenced nodes. The idea being, that we can use those UUIDs to determine the client-side node IDs for them so that we write the correct ids to the book table in the database.
How should we handle those references? When we receive one book node, we may not have all its reference available yet at this point, and we need to somehow handle those dependencies. Any help on how to approach this is much appreciated.
Comment #4
GrimreaperHi @jurgenhaas,
Thanks a lot for pushing this!
I wonder if this should be Entity Share sub-modules or dedicated Drupal projects?
So there will a jsonapi_book module which will provide what you have added done "drupal_internal__book".
And then in Entity Share, a JSON API Extras field enhancer that will convert or add entries like "nid_uuid" like done for block field https://git.drupalcode.org/project/entity_share/-/blob/8.x-3.x/src/Plugi...
And so add an importProcess Plugin that will parse these entries and import it like https://git.drupalcode.org/project/entity_share/-/blob/8.x-3.x/modules/e....
Warning about the current code of this importer plugin: #3265613: Infinite Loop in link field, block field and embedded entity importer plugins.
What do you think?
Comment #5
jurgenhaasYes, that sound reasonable. I'll have a detailed look into your links and will come back if any further questions arise.
The plugin should go into entity_share_client then?
Comment #6
GrimreaperThe field enhancer plugin should go into entity_share and the import processor plugin should go into entity_share_client.
Comment #7
jurgenhaas@Grimreaper this is now in a workable state!
It only requires the processor plugin
BookStructureImporter, which needs to also hook into thepost_entity_savestage to write to the book table after the entity and all its references have been saved. For that, I need to store the book records in the runtime import context. The way I've done it now is not ideal, but the API does not provide any better way yet. Maybe that should be added to the context?The field enhancer plugin is not required because that is already covered by the JSON:API Book module that I've written.
I've tested this in a fairly large environment, and it is working just fine. In the end, the code is fairly minimal, but it enhances the framework for books just nicely.
Comment #8
GrimreaperComment #9
Grimreaper@jurgenhaas: very good job!
Please see my review points.
Also this would need tests.
I can help you if you need input on how to write the tests but you may already see the tests for the other importer plugins.
Comment #10
jurgenhaasComment #11
GrimreaperComment #12
jurgenhaasIt's probably a bit late, but I have now incorporated your in-code comments from the MR and did also some code cleanup.
Wanted to start on the test but struggle with that and probably need your help. The challenge is that we need to have the module
json_api_bookon the server side of entity_share to inject the book related extra data which then needs to be tested on the client side.How would youy go about that for testing then?
Comment #13
GrimreaperHi @jurgenhaas,
Glad to see your are still active on this issue.
You can add json_api_book in the require-dev of the composer.json.
In a dedicated test which could inherit of EntityShareClientFunctionalTestBase, enable json_api_book in addition in this test and take inspiration from BasicFieldsTest and/or TaxonomyEntityReferenceTest or other tests about entity reference fields to see how the setup and the tests are done.
See tests like EmbeddedEntityTest when configuring additional import processors and/or json api extras enhancers are required.
Are those guidelines ok for you?
Comment #14
jurgenhaasComment #15
jurgenhaasThanks @Grimreaper this is great advise. But I still don't get it completely. I have an idea how you've built all those abstract classes to build your test data, but I don't easily find out how I should define my test data.
I'm sure you could do that fairly quickly with your knowledge of all that background. I've started building
\Drupal\Tests\entity_share_client\Functional\BookStructureImporterTestin the MR and you can find the information of what is needed in\Drupal\Tests\entity_share_client\Functional\BookStructureImporterTest::getEntitiesDataArraywhere I have included the data structure of the node, its parent node and its book node. If you could turn that into the correct data array, I may well get the rest of the test completed.Comment #16
GrimreaperThanks for your hard work and to have prepared the test class.
I will try to give it a look.
Comment #17
GrimreaperPlease check the structure of the nodes in getEntitiesDataArray if it is ok, especially the p* and depth.
The checker_callback methods have to be implemented like in ContentEntityReferenceTest or TaxonomyEntityReferenceTest.
If too time consuming, at least ensure the book data is ok and I will try to finish the test.
Thanks :)
Comment #18
jurgenhaasI have checked and updated the data structure and also implemented the checker callback, hopefully in the right way? But the structure looks ok to me now.
The test is failing with this error:
Would be great if you could get this fixed.
Comment #19
GrimreaperThanks!
Ok, I will try to test that and finish the test.
I am currently on #3265613: Infinite Loop in link field, block field and embedded entity importer plugins and will give a try here after.
Comment #22
GrimreaperGot it!
The problem was that during the entity creation, the book structure requires the entity ID while it does not exist yet.
So need to create the entities in prepareContent().
I gave a look at the tests in the core Book module to see how to create a book structure programmatically.
I also made simplification of the plugin to only import the parent and by recursion it will be ok.
@jurgenhaas can you please give a look at the new MR (rebased) to see if it is ok for you?
Without response I will merge next week.
Comment #23
jurgenhaasThis is looking great. Your simplication of the code is amazing, thanks a lot for that.
Comment #25
GrimreaperThanks for the quick feedback.
Merged!
Only #3340619: D10: update dev dependency Simple Oauth to ~5.2 before next release.
And #3107278: Call to a member function getAlias on null before stable release.
Comment #26
jurgenhaasThat's amazing, great work!
We have a very complex environment with books where we run real tests asap again. They will run a couple of days just because of the volume, but we will quickly see if all books still come across properly. If there was any issue, I'd open a new one here to address it directly. But there shouldn't be any.