[DFC Orders] List products to import on screen

[dacook]

ℹ️ Funded Feature. Please track ALL ASSOCIATED WORK under the associated tracking code #11678 DFC Orders

What? Why?

• Implements main requirement of [DFC Orders] List products to import on screen #12301
• There's some other notes on that issue that aren't covered yet.

It's not pretty, but it works!

What should we test?

1. Visit /admin/product_import
2. Scroll to bottom, enter URL of a DFC products endpoint, eg:

• https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts
• https://staging.openfoodnetwork.org.au/api/dfc/enterprises/{id}/catalog_items (where id is the db id of an enterprise that your user has management role for)

3. Select enterprise to import into
4. Click "Preview"
5. Summary should be shown with correct info.
6. De-select some products to avoid importing them. Count the number of selected products.
7. Click import. The correct number of products should be imported.
8. Return to the import preview screen (or click back button). It should reload, showing which products are now existing in the db.

Release notes

Changelog Category (reviewers may add a label for the release notes):

• User facing changes
• API changes (V0, V1, DFC or Webhook)
• Technical changes only
• Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

[mkllnk]

Nice work! I generally approve. There are just some missing translations and I'm wondering if I'm alone in my "use Rails default routes when possible" thinking.

[mkllnk]

I see the temptation to call the action import but I would prefer to go with Rails defaults, convention over configuration. In this domain, I would say that we create a Product Import. Following that logic, the index action should probably be a new action. 🤔 But the form on the product import screen could also be seen the the new form.

What do you think?

[dacook]

I didn't think a lot about it, but thought that Rails' resource/collection model didn't fit well in this case, so simply opted to copy the existing action name on the ProductImportController.
I suppose another way to look at it is an action on the products resource itself, ie: /admin/products/import or /admin/products/batch_create.

Ultimately I don't think it's worth thinking about anymore ;)
But it would be good to have a third opinion.

[dacook]

To further clarify my thinking, I didn't think there was much "cost" at all using a non-standard action name.
And I think to "create a Product Import" sounds too abstract, which makes it harder to see what it's for.

[rioug]

When possible I prefer to stick with the defaults Rails action, but I think in this case David's solution is acceptable. After all, we can't really refer to Updating or Deleting a "Product Import", we don't have a tangible model for "Product Import".

[rioug]

Looks good 👍

[rioug]

When possible I prefer to stick with the defaults Rails action, but I think in this case David's solution is acceptable. After all, we can't really refer to Updating or Deleting a "Product Import", we don't have a tangible model for "Product Import".