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

[API] product association type #11319

Merged
merged 10 commits into from
Apr 7, 2020

Conversation

oallain
Copy link
Member

@oallain oallain commented Apr 1, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets partially #11250
License MIT
  • adding_product_association_type.feature
  • browsing_product_association_types.feature
  • deleting_multiple_product_association_types.feature
  • deleting_product_association_type.feature
  • editing_product_association_type.feature
  • filtering_product_association_types.feature
  • product_association_type_unique_code_validation.feature
  • product_association_type_validation.feature

@oallain oallain requested a review from a team as a code owner April 1, 2020 19:56
@CoderMaggie CoderMaggie added API APIs related issues and PRs. Feature New feature proposals. labels Apr 2, 2020
@lchrusciel lchrusciel mentioned this pull request Apr 2, 2020
12 tasks
@oallain oallain force-pushed the api-product-association-type branch from 381af73 to 5084275 Compare April 3, 2020 11:34
@oallain oallain changed the title [WIP][API] product association type [API] product association type Apr 3, 2020
Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

Hey Olivier,

thanks for your effort ;)

*/
public function iNameItIn(string $productAssociationTypeName = null, string $localeCode = 'en_US'): void
{
$this->client->updateRequestData(['translations' => [$localeCode => ['name' => $productAssociationTypeName, 'locale' => $localeCode]]]);
Copy link
Member

Choose a reason for hiding this comment

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

Can we break it into multiple lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, of course.

@@ -10,7 +10,7 @@ Feature: Deleting multiple product association types
And the store has also a product association type "Accessories"
And I am logged in as an administrator

@ui
@ui @api
Scenario: Deleting multiple product association types at once
Copy link
Member

Choose a reason for hiding this comment

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

We have decided to skip these features because we weren't sure how should we process multiple deletions, but thanks for your proposal! ;)

}

/**
* @Then this product association type name should be :name
Copy link
Member

Choose a reason for hiding this comment

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

Same as https://github.com/Sylius/Sylius/pull/11319/files#diff-9f9847e886745cd71df41b26b4816ae1R166

Suggested change
* @Then this product association type name should be :name
* @Then /^(this product association type) name should be "([^"]+)"$/

}

/**
* @Then the code field should be disabled
Copy link
Member

Choose a reason for hiding this comment

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

During implementing new steps, we have decided, that we would like to slightly adjust these steps to something like https://github.com/Sylius/Sylius/pull/11178/files#diff-32a4b0d263dad520cfb84a0704f82849R24

@lchrusciel lchrusciel merged commit b9b91cc into Sylius:master Apr 7, 2020
@lchrusciel
Copy link
Member

Thanks, @oallain! 🎉

pamil added a commit that referenced this pull request Apr 9, 2020
This PR was merged into the 1.6-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | part of #11250, improvements from #11319
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.6 or 1.7 branches (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set
-->


Commits
-------

ce9964f [API] Association types improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants