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][Administrator] refactor endpoints for admin and admin avatar #11776

Merged

Conversation

AdamKasp
Copy link
Contributor

Q A
Branch? 1.8
Bug fix? yes
New feature? no
BC breaks? no
Related tickets #11250
License MIT

@AdamKasp AdamKasp requested a review from a team as a code owner August 31, 2020 10:45
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Aug 31, 2020
@@ -211,4 +211,13 @@ private function mergeArraysUniquely(array $firstArray, array $secondArray): arr

return $firstArray;
}

private static function prepareSection(?string $section): string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function should be deleted after refactor all new API endpoints, for now only refactored endpoint have section prefix, without this logic a have problems with pathnames while testing.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, we can leave it for the future, to give possibility of using this class without any section

AbstractBrowser $client,
SharedStorageInterface $sharedStorage,
string $resource,
string $section = null
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
string $section = null
?string $section = null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix it in the next PR.

@AdamKasp AdamKasp force-pushed the api-refactor-admin-and-avatar-endpoints branch from 38668a3 to 81bebec Compare August 31, 2020 15:57
@lchrusciel lchrusciel merged commit ec815e6 into Sylius:1.8 Sep 1, 2020
@lchrusciel
Copy link
Member

Thank you, Adam! 🥇

lchrusciel added a commit that referenced this pull request Sep 1, 2020
This PR was merged into the 1.8 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.8
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Related tickets | part of #11250, based on #11776 
| License         | MIT

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

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

dc45032 [API] refactor endpoint for api resources
@AdamKasp AdamKasp deleted the api-refactor-admin-and-avatar-endpoints branch July 19, 2021 06:20
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants