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

Simplify ingredient creation #2171

Merged
merged 3 commits into from
Aug 12, 2021

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Aug 11, 2021

What is this pull request for?

Removes the abstract_class from Alchemy::Ingredient

This caused a lot of issues during creating an ingredient record, because Rails holds a lot of precautions for abstract classes.

Since this is not necessary for STI to work and we already have validations of all necessary attributes this can safely be removed allowing us to simplify the process of creating an ingredient record.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@tvdeyen tvdeyen added this to the 6.0 milestone Aug 11, 2021
@tvdeyen tvdeyen requested a review from a team August 11, 2021 15:39
@tvdeyen tvdeyen force-pushed the simplify-ingredient-creation branch from 7d781bc to 3f2b572 Compare August 11, 2021 16:00
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

👍 👍

@tvdeyen tvdeyen force-pushed the simplify-ingredient-creation branch from 3f2b572 to 1120e48 Compare August 11, 2021 16:13
@tvdeyen tvdeyen enabled auto-merge August 11, 2021 16:14
This causes a lot of issues during creating an ingredient
record, because Rails holds a lot of precautions for abstract
class. Since this is not necessary for STI to work and we
already have validations of all necessary attributes this
can safely be removed allowing us to simplify the process
of creating an ingredient record.
@tvdeyen tvdeyen force-pushed the simplify-ingredient-creation branch from 1120e48 to f5ff098 Compare August 12, 2021 11:37
Now that we removed abstract_class from the ingredient class
we can use all the Rails defaults to create an ingredient instances.
In the element editor with have a feature that creates ingredients
that are defined but not created yet. Before we removed the abstract
class from Ingredient we needed to create the ingredient on the class
instead of the collection. Since the collection is eager loaded it has
to be updated with the newly created ingredient in order to prevent
the ingredient to be created twice (leading to unique index violation
errors).
@tvdeyen tvdeyen force-pushed the simplify-ingredient-creation branch from f5ff098 to aa2ccd0 Compare August 12, 2021 11:37
@tvdeyen tvdeyen merged commit 76a39a4 into AlchemyCMS:main Aug 12, 2021
@tvdeyen tvdeyen deleted the simplify-ingredient-creation branch August 12, 2021 11:43
tvdeyen added a commit to tvdeyen/alchemy_cms that referenced this pull request Aug 27, 2021
This was necessary before we started to use Rails' data store feature in
AlchemyCMS#2171

This currently leads to weird bugs around active record dirty falsy thinking
the attribute set is not dirty, leading
to the data store not being saved.
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.

2 participants