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.

Sign up for GitHub

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

Jump to bottom

Fix Order comment REST endpoint route param #2750

Merged
merged 2 commits into from
Nov 27, 2022
Merged

Fix Order comment REST endpoint route param #2750

merged 2 commits into from
Nov 27, 2022

Conversation

elidrissidev
Copy link
Member

@elidrissidev elidrissidev commented Nov 20, 2022 •
edited
Loading

Description (*)

In #2315, I mistakenly left out the param name as comment_id instead of id, which fails when calling $this->_getLocation() because it expects the param name to always be id.

Related Pull Requests

  1. See Order add comment functionality in REST API #2315
  2. See Backport: #2315 #2746

Manual testing scenarios (*)

  1. When adding comment to an Order via the REST API, the request should succeed and the the response should contain a valid Location header.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: Sales Relates to Mage_Sales label Nov 20, 2022
@sreichel
Copy link
Contributor

#2315 has been backported, can you change target to 19.4.x?

@sreichel sreichel mentioned this pull request Nov 25, 2022
Merged
@elidrissidev elidrissidev changed the base branch from 20.0 to 1.9.4.x November 25, 2022 09:33
@github-actions github-actions bot added Component: AdminNotification Relates to Mage_AdminNotification Component: Adminhtml Relates to Mage_Adminhtml Component: Admin Relates to Mage_Admin Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Authorizenet Relates to Mage_Authorizenet Component: Backup Relates to Mage_Backup Component: Bundle Relates to Mage_Bundle documentation environment htaccess Mage.php Relates to app/Mage.php phpstan Component: Sales Relates to Mage_Sales and removed Component: Sales Relates to Mage_Sales Mage.php Relates to app/Mage.php Component: AdminNotification Relates to Mage_AdminNotification Component: Api2 Relates to Mage_Api2 htaccess Component: Backup Relates to Mage_Backup Component: Admin Relates to Mage_Admin Component: Adminhtml Relates to Mage_Adminhtml Component: Authorizenet Relates to Mage_Authorizenet labels Nov 25, 2022
@github-actions github-actions bot removed Component: Bundle Relates to Mage_Bundle Component: Api PageRelates to Mage_Api phpstan labels Nov 25, 2022
@elidrissidev
Copy link
Member Author

@sreichel Done

sreichel
sreichel approved these changes Nov 26, 2022
View reviewed changes
Copy link
Contributor

@sreichel sreichel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Not tested.

@fballiano fballiano merged commit 879caab into OpenMage:1.9.4.x Nov 27, 2022
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 879caab. ± Comparison against base commit a559a55.

@elidrissidev elidrissidev deleted the fix/rest-order-add-comment branch November 27, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sreichel sreichel sreichel approved these changes

@fballiano fballiano fballiano approved these changes

Assignees
No one assigned
Labels
Component: Sales Relates to Mage_Sales documentation environment
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@elidrissidev @sreichel @fballiano