I remember at the time this was done we were very annoyed that it was the best solution we could come up with, but it's likely that if we revisit it we can come up with something better. Possibly we were being too stubborn at the time about not using Drupal-specific paradigms in the API or Harvest specs like "public://"? But it seems like yes you should be able to put either an absolute file path or a public:// url in a harvest and it should work.

The only place where file_public_base_url is mentioned in code is Drupal\harvest\Transform\ResourceImporter::saveFile(), which is only used by the sample_content harvest.

I experimented with commenting out the file_public_base_url setting in a local ddev environment, and then building a site and doing the sample content import.

It turns out we might not actually need this setting to be explicitly set before showing sample content.

Some git archaeology tells us that the comment about file_public_base_url comes from 2ab75ca (2019), and since then much refactoring of ResourceImporter has happened.

In terms of testing, ResourceImporter has the @codeCoverageIgnore annotation, so we don't know what covers this behavior. Removing this annotation, we find that it's not covered by any test.

Recommendations:

• Remove this special configuration case from settings.dkan.php in the ddev-dkan. https://github.com/GetDKAN/ddev-dkan/blob/main/misc/settings.dkan.php
• Add a test that covers installing and importing sample content without this configuration. This could be within the DKAN DDev add-on, or it could be within DKAN core.
• Unit test coverage of ResourceImporter.
• Bonus points: Move Drupal\harvest\Transform\ResourceImporter into the sample_content module, since this transform is used for sample content.

@paul-m I agree with the first three recommendations, but I think the ResourceImporter is potentially useful for other default content modules people might implement for their own sites, and also is one of the few examples of a transform class, so I would keep it where it is.

Review question from @janette Does it hurt to remove the config?