Support for read of Image in Cell

[raziel057]

Support for read of Image in Cell - Related to #4014

This is:

• a bugfix
• a new feature
• refactoring
• additional unit tests

Checklist:

• Changes are covered by unit tests
  • Changes are covered by existing unit tests
  • New unit tests have been added
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
• Documentation is updated as necessary

Why this change is needed?

When an image is inserted in a cell in an XLSX document, PhpSpreadsheet is currently not able to read it. This PR allow to collect those images in order to find them when calling $objWorksheet->getDrawingCollection()

The code have been tested - As shown bellow:

When I import the file with image inserted in the Cell, I have it now in $objWorksheet->getDrawingCollection():

As an improvement, maybe we should introduce a DataType::TYPE_DRAWING and adapt the code accordingly to allow the user to get the Drawing object when calling getValue() on the cell?

[oleibman]

You will need to fix the phpcs, php-cs-fixer, and phpstan errors. After those, the unit test errors. After that, you need some formal unit tests. Finally, you've addressed only the read side. It isn't absolutely required, but I would be happier with a change which addressed both reading and writing (which I will admit is probably much harder).

I do not know if TYPE_DRAWING is a good idea or not. I'll think about it when this PR is in better shape.

[oleibman]

It looks like you have only one test failure, but it was caught during multiple tests. When a formula has an array result which spills into cells which already have values, Excel also uses the vm attribute for the cell. I think it is probably okay for you test for a vm attribute and no child f tag when deciding if it's an image. There may be more to it, but that will at least get you over one hurdle for now.

[raziel057]

I reworked the implementation as I could see that images are defined as Relationship in richData\_rels\richValueRel.xml.rels.

I didn't had the time to work on the Write part, but I think it could be done in another PR.

[raziel057]

Not sure why the coverage decreased (-0.04%) as I created a Unit Test that cover all the code that have been added. I'm not able to see the details https://coveralls.io/builds/75720737/source?filename=src%2FPhpSpreadsheet%2FReader%2FXlsx.php#L791

[oleibman]

Thank you for adding the tests. Decreasing code coverage is not necessarily a reason for us to proceed with a change. However, in this case, it is a showstopper. The Coveralls report shows that most of the code you added is not being exercised by the unit test suite. In particular:

• You added lines 787-799 in Reader/Xlsx, but lines 791-795 are never executed.
• You added lines 956-977, but lines 957-975 are never executed.

So, either the bulk of your new code is unneeded, or it is inadequately tested. Please see what you can do about that. I have some ideas about how to proceed once you've attended to this.

[raziel057]

Ok I forgot the Test suffix in DrawingInCellTest, so it was not taken when running the test suite. It should be fine now.

[kboumedal]

Thank you for this PR !
Any update on merging it ?