Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

create bodytype json #357

Merged
merged 3 commits into from
Sep 11, 2023
Merged

create bodytype json #357

merged 3 commits into from
Sep 11, 2023

Conversation

ankitdas13
Copy link
Contributor

  • Changed default body type urlencode to json while create order

@ankitdas13 ankitdas13 added Tested Tested label for BVT and removed Tested Tested label for BVT labels Aug 30, 2023
@ankitdas13 ankitdas13 requested a review from ramth05 August 30, 2023 12:21
class Order extends Entity
{
/**
* @param $id Order id description
*/
public function create($attributes = array())
{
$attributes = json_encode($attributes);
Copy link
Contributor

Choose a reason for hiding this comment

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

@ankitdas13 , Is there any tests needs to be updated for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramth05 we dont have to update any where for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramth05 added negative test cases for create order

@ankitdas13 ankitdas13 requested a review from ramth05 September 4, 2023 06:43
/**
* @covers \Razorpay\Api\Order::create
*/
public function testSetHeaderJson(){
Copy link
Contributor

Choose a reason for hiding this comment

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

start { from new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{ now started from new line


$this->assertTrue(is_array($data->toArray()));

}catch(Error $e){
Copy link
Contributor

Choose a reason for hiding this comment

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

catch must be start from new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

catch now started from new line

private function payload(){
$date = new \DateTime();
$receiptId = $date->getTimestamp();
return array(
Copy link
Contributor

Choose a reason for hiding this comment

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

@ankitdas13 , please use [] for array and do the proper formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed array() method to []

{
$payload = $this->payload();

$attribute = json_encode($payload);
Copy link
Contributor

Choose a reason for hiding this comment

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

line space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove space between $attributes & try

@ankitdas13 ankitdas13 requested a review from ramth05 September 8, 2023 08:34
@poojarazor poojarazor self-requested a review September 8, 2023 12:46
Copy link

@poojarazor poojarazor left a comment

Choose a reason for hiding this comment

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

LGTM

@ankitdas13 ankitdas13 added TestingNotRequired TestingNotRequired label for BVT and removed Tested Tested label for BVT labels Sep 8, 2023
@ankitdas13 ankitdas13 merged commit b29754f into master Sep 11, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
TestingNotRequired TestingNotRequired label for BVT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants