openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.09k stars 708 forks source link

[BUU] Re-organise products_v3 feature specs #12497

Closed dacook closed 4 days ago

dacook commented 1 month ago

What we should change and why (this is tech debt)

As development of this new screen continues, the one big file is becoming unwieldy, so we would like to split it up. Suggested split:

Note that the folder products_v3 would be renamed to simply products after the old products screen is removed.

Note that this doesn't include other products specs, like the New Product and Edit Product screens. They may also be re-organised.

Dependencies

sigmundpetersen commented 3 weeks ago

What about create_specs instead of new_specs ? "new" is a bit ambiguous

chahmedejaz commented 3 weeks ago

What about create_specs instead of new_specs ? "new" is a bit ambiguous

I think @dacook used the Rails convention here where new represents form fields that are filled to create an object in DB by using the data that was filled. That's why in context of the above,new_specs maybe sounds better here as we will be writing specs that will be taking data in the form to create product records. 😅

dacook commented 3 weeks ago

I agree "new" is more ambiguous, because it could also be describing the new products screen vs the old.

For "system" (feature) specs, the focus should be the user's point of view, not necessarily describing the code. The screen it tests is called "New product" (in default locale), so "new" makes sense, but I suppose the goal is to "create" a product. At the end of the day, we just want to avoid ambiguity, so I think "create" is a good idea, but I don't care that much :)

chahmedejaz commented 2 weeks ago

Hi @filipefurtad0 - When it's ready for me to take on the work, please let me know. If you think you should complete all, then no worries. I'll unassign myself in that case. Thanks 🙂

dacook commented 1 week ago

Hi @chahmedejaz , I've merged existing PRs that touch the products_v3 specs, so you may proceed with the rest of this issue now.

chahmedejaz commented 4 days ago

Update: PR is ready for review now :)