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

Deprecate dangling option_values and duplicated routes #4385

Conversation

waiting-for-dev
Copy link
Contributor

@waiting-for-dev waiting-for-dev commented May 26, 2022

Up until now, we were allowing the creation of option_values without an associated option_type. It makes no sense from the domain logic perspective. Ref #3309 & #3517.

On top of that, we're deprecating the nested version for member routes, as there's no need to fetch the parent id for those.

It makes no sense to have dangling option_values without an associated
option_type.

Ref solidusio#3309 & solidusio#3517
@waiting-for-dev waiting-for-dev requested a review from kennyadsl May 26, 2022 06:16
@kennyadsl
Copy link
Member

I asked for some context here, #3517 (comment). It's quite old so not sure we can get anything but I thought it was worth trying.

@waiting-for-dev
Copy link
Contributor Author

I asked for some context here, #3517 (comment). It's quite old, so not sure we can get anything, but I thought it was worth trying.

It looks like the referenced PR didn't introduce the behavior. It only added a test to make it explicit and fixed an issue when option_type is nil. As per #3309, the behavior has been that way because it used to be the default for all belongs_to associations on Solidus.

@waiting-for-dev
Copy link
Contributor Author

By the way, for some reason, CircleCI result is not being reported, although it was successful. See https://app.circleci.com/pipelines/github/nebulab/solidus?branch=waiting-for-dev%2Fduplicated_option_value_endpoints&filter=all

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Ah got it, thanks @waiting-for-dev!

There's no need to fetch the parent id for those routes.
@waiting-for-dev waiting-for-dev force-pushed the waiting-for-dev/duplicated_option_value_endpoints branch from d9583bf to 7c49645 Compare May 27, 2022 10:59
@waiting-for-dev waiting-for-dev merged commit 3c72462 into solidusio:master May 27, 2022
@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/duplicated_option_value_endpoints branch May 27, 2022 11:23
waiting-for-dev added a commit to nebulab/solidus that referenced this pull request May 30, 2022
As in solidusio#4385:

- We keep shallow member routes. We deprecate nested member routes.
- We keep creation nested route. We deprecate creation shallow route.
- We keep both shallow and nested collection routes (more flexibility
for storefronts).
cpfergus1 pushed a commit to cpfergus1/solidus that referenced this pull request Aug 25, 2022
As in solidusio#4385:

- We keep shallow member routes. We deprecate nested member routes.
- We keep creation nested route. We deprecate creation shallow route.
- We keep both shallow and nested collection routes (more flexibility
for storefronts).
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 21, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 21, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 27, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 12, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 14, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref solidusio#4385.
kennyadsl added a commit that referenced this pull request Apr 18, 2023
Add a NOT NULL validation at the database level on top
of removing the existing deprecation warning.

Ref #4385.
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