Skip to content

Conversation

@piyyu
Copy link

@piyyu piyyu commented Dec 25, 2025

What? Why?

The "Import multiple products" button was linking to a non-existent route
(/admin/products/import), which resulted in a "Not Found" error when clicked,
especially visible when there were no products.

This change updates the button to use the correct
admin_product_import_path, ensuring it correctly navigates to the product
import page.

What should we test?

  • Login as an admin user
  • Go to Admin → Products with no products present
  • Click Import multiple products
  • Confirm the product import page loads correctly
  • Verify the button still works when products already exist

Release notes

Changelog Category:

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

Dependencies

None

Documentation updates

None

@sigmundpetersen sigmundpetersen moved this from All the things 💤 to Code review 🔎 in OFN Delivery board Dec 25, 2025
@spreadFact
Copy link

Gracias — esto arregla el enlace roto y se ve bien. Probé el flujo localmente: en Admin → Products con la lista vacía, el botón "Import multiple products" ahora abre correctamente la página de importación, y también funciona cuando ya hay productos.

Pequeña sugerencia: ¿podrías añadir una spec de sistema/feature que verifique el caso sin productos (visitar Admin → Products, hacer click en el botón y comprobar que se navega a admin_product_import_path)? Aparte de eso, LGTM — apruebo.

@chahmedejaz chahmedejaz self-requested a review December 26, 2025 11:29
Copy link
Collaborator

@chahmedejaz chahmedejaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks good to me. 🎉

Now we will wait for the 2nd review before moving to the manual testing, @piyyu. Thanks

@piyyu
Copy link
Author

piyyu commented Dec 26, 2025

@spreadFact thanks for the suggestion! Happy to add a system spec in a follow-up if needed 👍

@mkllnk mkllnk added the user facing changes Thes pull requests affect the user experience label Jan 2, 2026
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Great work.

expect(page).to have_link(class: "button", text: "New Product", href: "/admin/products/new")
expect(page).to have_link(class: "button", text: "Import multiple products",
href: "/admin/products/import")
href: admin_product_import_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it would be okay to use the explicit string. Otherwise we are not testing much here and just mirroring the code.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, do as you wish,if you need some thing more ,

@mkllnk mkllnk moved this from Code review 🔎 to Test Ready 🧪 in OFN Delivery board Jan 2, 2026
@spreadFact
Copy link

spreadFact commented Jan 2, 2026 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy-test user facing changes Thes pull requests affect the user experience

Projects

Status: Test Ready 🧪

Development

Successfully merging this pull request may close these issues.

On empty product list, button to import multiple products triggers an error

5 participants