Mapping to books

I think it is technically possible to import data for books (without architectural changes in Feeds).

Book data is stored in a separate table, namely the table "book". When I look at the code of the book module, it seems it is coded in a quite customly manner: no fields are used and the book data is also not entity data.

I think the best way forward is creating a new target plugin that is based on \Drupal\feeds\Plugin\Type\Target\TargetBase and which sets properties of 'book'. See book_schema() for which properties there are for book.

Looking at the code, it looks like some properties are automatically set, like 'nid', for example. At first glance, useful properties to expose are at least:

• 'bid', which represents the node ID of the top-level page. If it is set to "new" the node itself is the top-level page.
• 'pid', which represents the parent node ID.
• 'weight'

Not sure about the others.

Will you work on this feature, @Harrietech? You assigned the issue to yourself. If not, you may need to hire a developer if this is an important feature to you.

I've created a mapping target for 'Book'.

Summary of the additions:

• Added a new mapping target "book". This target is only available when the Book module is installed. Other than other FeedsTarget plugins, this is not a field target, because the book data from the book module is not a field.
• Added unit tests the prepareValues() method.
• Added kernel tests for importing into book. The tests check the following:
  1. Inserting a node for an existing book.
  2. Updating a node, assign it to an existing book.
  3. If the imported node gets into a book when only the parent node is specified.
  4. Removing a node from a book.
  5. If the node gets assigned to a book on a subsequent import if the book is not available during the first import.
  6. If the node gets assigned to a parent node on a subsequent import if the parent node is not available during the first import.
• A new service called "Entity finder" (feeds.entity_finder), inspired by the EntityReference target. Maybe that target should be updated to use the new service.
• The FeedsTarget plugin "book" contains the additional methods isMutable() and isEmpty(). This is because locally I was creating this feature on top of the Entity ID target feature (#2989279: Add a target to entity ID) and that feature requires these new methods. I hope to finish off that one soon too, so both can be committed.

Coding standard fixes.

"scalar" is not valid type hint, so it seems. I tried to fix the coding standard violation in an other way.

This saved me a a lot of time. I can confirm that this works as advertised using different sources for finding the book and parents including feed item.guid.

I tested updated module on a site with existing feeds and updated version works correctly. I think I accidentally created fork https://git.drupalcode.org/issue/feeds-3073275 instead of branch.

I updated the book target to make it compatible with the updated Entity service (which was committed yesterday in #3206365: Add an EntityFinder service).

It would be good to test the updated version before commit. I did tests the changes myself and so far I think it still works.

The code changes are now in an issue branch instead of a patch.

I set up testing instance using DevPanel here http://dev-13c1d72a-13c1d77f-13c1d783.app.devpanel.com/, but I realized that I do not have good data example for books import. Are there any recommendations on how to set this data so this feature could be tested?

@irinaz
Books in Drupal are hierarchical nodes. Each book page refers to the book that they are in and what their parent node is.

I have tested importing books myself with the following CSV data:

title,book,parent
Section 1,Book 1,Book 1
Chapter 5,Book 1,Section 2
Chapter 1,Book 1,Section 1
Chapter 2,Book 1,Section 1
Chapter 3,Book 1,Section 1
Section 2,Book 1,Book 1
Chapter 4,Book 1,Section 2
Paragraph 1,Book 1,Chapter 1

Note that Chapter 5 is defined before Section 2 is defined: in this case, a second import would be needed to put "Chapter 5" at the right spot in the book.

And this is a screenshot of how I setup the mapping:

I already tested with this configuration and data and for me that worked correctly. It would be more interesting to test with other configurations. For example: I have not tested what happens if you do map to "Book outline (book): Parent item", but not to "Book outline (book): Book". And vice versa. Also, I've not tested other fields to reference by (I only tested referencing data by title, not by ID, UUID or "Feeds item: GUID"). These cases should be covered by the automated tests, but I think are still useful to test manually because tests could potentially also have bugs.

Tested import with weight, works great. If "Book outline (book): Book" is not mapped then outline is not created correctly. Maybe "Book outline (book): Book" should be required.

Reviewed the code quickly and WOW. @MegaChriz has such clean code!!! :)

Regarding:

Maybe "Book outline (book): Book" should be required.

I think it would cause less confusion if it was required. What's the use case is using "Book outline (book): Parent item"?

Seems like @MegaChriz mainly wants more manual testing?

@Kristen Pol, books are used for hierarchy structure "out of the box". Top level page in a book does not have parent. I think that adding this feature "as is" will be helpful. After your review do we move it to RTBC?

I only looked at the code and didn't do any testing. I assume more is needed? Or is what you did last year sufficient? Also, there is a ton of code and I don't claim to understand it all so, if you want a deep code review, then you could ask around.

Marking this as "Needs work" because of the feedback in #14:

If "Book outline (book): Book" is not mapped then outline is not created correctly.

Summary of the changes:

• You can now configure mapping for "Top-level page". When 1 is passed, Feeds will attempt to create a new book of the item.
• When mapping to both book_id and parent_id, the parent node will be searched within the given book.
• Test coverage for both two new features is added and existing unit tests are updated.

I will try to review and test =)

Working testing this issue :)

Wow. I tested the MR and it looks really good.
I made a very small change related to unused statements and deprecated code but for me is RTBC.

The code is merged! Thanks everyone for testing and reviewing!