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

#865: add getParams/setParam to RequestInterface #1053

Merged
merged 2 commits into from
Mar 6, 2015
Merged

#865: add getParams/setParam to RequestInterface #1053

merged 2 commits into from
Mar 6, 2015

Conversation

ashsmith
Copy link

As per issue #865, the getParams and setParams method is missing from the RequestInterface. Which are methods relied upon heavily in app/code/Magento/*.

@otoolec
Copy link
Contributor

otoolec commented Feb 20, 2015

@ashsmith Thanks for providing this pull request. In order for us to accept the PR we would need the Travis CI tests to pass though.

  • Can you fix the broken unit tests? I've provided a fix for one of them as a PR to your branch. Please identify other failing tests and provide similar fixes.
  • Can you modify all implementations of the RequestInterface to conform to the changes you've made? For example the Magento\Framework\App\Console\Request class needs to have the new methods added.

After making these changes, please make sure that there aren't any other issues identified by TravisCI.

@anupdugar anupdugar added the PS label Feb 20, 2015
@ashsmith
Copy link
Author

Working on fixing the failing tests. Didn't realise quite how many would be affected by this change.

Ash Smith added 2 commits February 20, 2015 17:23
…thub.com:ashsmith/magento2 into 865-add-getParams-setParams-to-request-interface

Conflicts:
	dev/tests/unit/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/SaveTest.php
@ashsmith
Copy link
Author

Okay, I've updated the tests. I'm getting one failure locally that I can't debug where Zend_Controller_Response_Http::__toString() is throwing an exception when it isn't expected to, but not providing anything useful to help debug. Hoping it's just an env issue.

@otoolec otoolec self-assigned this Feb 24, 2015
@otoolec otoolec added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Feb 24, 2015
@otoolec
Copy link
Contributor

otoolec commented Feb 27, 2015

@ashsmith I've resolved the merge conflicts that exist and am submitting an internal PR for your code. This should allow us to accept your PR next week!

I didn't see any errors related to Zend_Controller_Response_Http::__toString() yet. Can you describe the scenario where you saw them? Was it from running unit tests locally?

@ashsmith
Copy link
Author

ashsmith commented Mar 2, 2015

@otoolec It was when running locally. I have a feeling it's to do with my local dev. I'll rebuild my vm tonight and run it again to be sure.

@ilol ilol added TECH and removed TECH labels Mar 2, 2015
vpelipenko pushed a commit to vpelipenko/magento2 that referenced this pull request Mar 5, 2015
…erface magento#865 magento#1053

Merge branch '865-add-getParams-setParams-to-request-interface' of git://github.com/ashsmith/magento2 into ashsmith-865-add-getParams-setParams-to-request-interface

Conflicts:
	dev/tests/unit/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/SaveTest.php
	dev/tests/unit/testsuite/Magento/Checkout/Helper/CartTest.php
	dev/tests/unit/testsuite/Magento/Checkout/Model/Type/OnepageTest.php
	dev/tests/unit/testsuite/Magento/Customer/Controller/Account/CreateTest.php
	dev/tests/unit/testsuite/Magento/Customer/Controller/Account/LoginPostTest.php
	dev/tests/unit/testsuite/Magento/Customer/Controller/Adminhtml/Index/IndexTest.php
	dev/tests/unit/testsuite/Magento/Customer/Controller/Ajax/LoginTest.php
	dev/tests/unit/testsuite/Magento/Customer/Model/Metadata/Form/FileTest.php
	dev/tests/unit/testsuite/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/LinkTest.php
	dev/tests/unit/testsuite/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/SampleTest.php
	dev/tests/unit/testsuite/Magento/Downloadable/Controller/Download/LinkTest.php
	dev/tests/unit/testsuite/Magento/Framework/App/Action/ActionTest.php
	dev/tests/unit/testsuite/Magento/Framework/App/Router/NoRouteHandlerTest.php
	dev/tests/unit/testsuite/Magento/Framework/Controller/Result/ForwardTest.php
	dev/tests/unit/testsuite/Magento/Framework/UrlTest.php
vpelipenko pushed a commit to vpelipenko/magento2 that referenced this pull request Mar 5, 2015
…erface magento#865 magento#1053

- Remove RequestInterface::getParam from obsolete_methods since it still exists
- Move getCookie to PhpEnvironment\Request
@magento-team magento-team merged commit ffbeaeb into magento:develop Mar 6, 2015
magento-team pushed a commit that referenced this pull request Apr 24, 2017
Implemented task:
- MAGETWO-66374 Add @api annotation
magento-engcom-team added a commit that referenced this pull request Nov 14, 2019
…reate new customer with the email of already existent user #1053

 - Merge Pull Request magento/graphql-ce#1053 from magento/graphql-ce:createCustomer-test-coverage
 - Merged commits:
   1. fe89ce4
magento-engcom-team pushed a commit that referenced this pull request Nov 14, 2019
…reate new customer with the email of already existent user #1053
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants