Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bye bye Spree] Bring models taxon and taxonomy from spree_core #5868

Merged
merged 8 commits into from
Sep 14, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Aug 6, 2020

What? Why?

Continues #4826
Here we move the following files to OFN:

  • core/app/models/spree/classification.rb MERGED WITH DECORATOR
  • core/app/models/spree/taxonomy.rb COPY
  • spec/models/spree/taxonomy_spec.rb COPY
  • core/app/models/spree/taxon.rb MERGED WITH DECORATOR
  • spec/models/spree/taxon_spec.rb MERGED WITH OFN VERSION

No code changes were made, here we only move the code from spree, adapt the specs if needed and improve the code a little with rubocop and transpec.

What should we test?

We need to verify if everything related to these changes is still working. I'd run the following verifications:

  • Backoffice Taxonomies feature - make sure you can manage taxonomies and taxons in them as before.
  • Make sure the changes in the taxonomy are reflected in the product categories both when editing the product and also when buying it in the shopfront.
  • Verify the filters by category in the shopfront are working

I am not sure what else to test related to taxonomies. Any other point we should check here?

Release notes

Changelog Category: Changed
Brought code needed in OFN from Spree so that we can make OFN independent of Spree.

@luisramos0 luisramos0 changed the title Taxonomies [Bye bye Spree] Bring models taxon and taxonomy from spree_core Aug 6, 2020
@luisramos0 luisramos0 self-assigned this Aug 6, 2020
@sauloperez
Copy link
Contributor

There are a few easy to fix Code Climate issues. Other than that 👍

@luisramos0
Copy link
Contributor Author

luisramos0 commented Sep 1, 2020

I am not sure about the "Useless assignment to variable - name"... so I dont change it.

I fixed the long lines 👍

@sauloperez
Copy link
Contributor

I'm moving it to test ready because it's a fairly simple merge and it looks 👍

@filipefurtad0 filipefurtad0 self-assigned this Sep 14, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 14, 2020
@filipefurtad0
Copy link
Contributor

Hi @luisramos0,

Checked the following:

i) Verified previously existing Taxons were working as correctly
ii) as Superadmin, created a new Taxon/Class. Created Subclasses.
iii) as Admin - created a new products with the newly created taxons - these appear alphabetically, when editing/creating a product. Placed them on the shopfront.
iv) as User - visited the shopfront and searched for the new product (using the search field) and filtered the results using the "Filter by", using the respective newly created taxon
v) Deleting Taxons is possible, as long as they are not attributed to a product.

Not introduced by this PR:

Error observed on test case ii) but no unusual behavior was seen on the app:

https://openfoodnetwork.slack.com/archives/G010VN35QU9/p1600101520000100

Error observed on test case v) if Taxons are attributed to a product:

https://openfoodnetwork.slack.com/archives/G010VN35QU9/p1600103192000600

image

I could not verify issue #5252 after staging this PR, so maybe this solves it? Would that make sense? I'll comment on that issue.

This is good to go! 🎉

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 14, 2020
@filipefurtad0
Copy link
Contributor

I verified #5252 now: it only affects main classes. Renaming sub-classes works both in back- and frontoffice.

@luisramos0
Copy link
Contributor Author

Nice, thanks Filipe. We could create an issue for that JS error (last_rollback), there's some rollback logic in the JS that maybe we dont need.

re #5252 👍

@luisramos0 luisramos0 merged commit 0d7b5cd into openfoodfoundation:master Sep 14, 2020
@luisramos0 luisramos0 deleted the taxonomies branch September 14, 2020 19:45
@filipefurtad0
Copy link
Contributor

Hey @luisramos0, thanks. Opened tech-debt #6030 concerning the removal of the rollback logic and issue #6033 on the observed errors while deleting taxons (bug-s4 I'd say).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants