When image source is a URL, store the URL for use during extraction.

[jarrettj]

When image source is a link store the link.

This is:

- [ ] a bugfix
- [x] a new feature

Checklist:

• Changes are covered by unit tests
• 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
• Documentation is updated as necessary

Why this change is needed?

Described in issue #1997.

[MarkBaker]

If you're storing the path in a separate property if it's a url, shouldn't the Writer also be modified to retrieve it correctly when an Xlsx file has been loaded and is then subsequently saved?

Does it really require a separate url property? Or would a flag indicating url be sufficient (with appropriate checking of that flag in the recipe?)

[jarrettj]

A flag alone would not work. If you check the updated recipe.md file you would see what I need to do. If it is a URL I would then get the image content using the URL. If it was a flag I would not know what the URL is. This change is to allow me to extract images from an excel file when the source is not a path but a URL.

Regarding the Writer, not sure if adding an image by URL is needed? Feel that would be a seperate PR maybe? I don't require that feature at the moment.

[MarkBaker]

A URL is a path though.... I'm not simply suggesting to discard the URL value, but to store it in the $path property, and use a simple flag to indicate whether $path contains a simple filepath or a url

[jarrettj]

Got it! Cool, I can do that. Let me rework it and commit. Thanks Mark.

And on the Writer, would it be fine to not change that?

Regards.
Jarrett

[MarkBaker]

You might want to test to confirm; but if you're using $path to store the source path whether it's a url or a file path, then the writer should simply write that path value back as it was in the original file

[MarkBaker]

Do you have a sample test file that you could use to write a basic Read/Write/Re-read test with assertions to verify the image source?

[jarrettj]

Hey, updated the code, onto the tests. Could you point me in the right direction on how to do the tests? Thanks.

[jarrettj]

Added a test for the reader, please have a look and see if it is sufficient. Thanks.

[oleibman]

I have a question about URLImageTest. It reads a spreadsheet which is part of the test suite, and then fetches an image from a URL. That URL is not part of the test suite, and its contents can change at any time. The test certainly works now, but it may fail in future. Are we okay with this? In fact, are we okay with making a URL request part of the test suite, which I believe has not been the case till now? Or should we be doing something like mocking the result for file_get_contents on line 29?

[jarrettj]

Hey @oleibman .

Good day.

And sorry for the delay, missed your reply. I can definitely look at doing that instead.

On a side note, how does the release cycles work? Thought there would be a new update by now already.

Regards.
Jarrett

[jarrettj]

Thanks for pulling this change in! Removes manual image uploads into our CMS. Keep up the great work :)