Skip to content

Change model Dynaform#125

Merged
velkymx merged 3 commits intodevelopfrom
bugfix/124
May 11, 2018
Merged

Change model Dynaform#125
velkymx merged 3 commits intodevelopfrom
bugfix/124

Conversation

@marcoAntonioNina
Copy link
Copy Markdown
Contributor

Change the field PRO_ID to process_id, and fields to lowercase

@marcoAntonioNina marcoAntonioNina requested a review from velkymx May 11, 2018 16:16
Copy link
Copy Markdown
Contributor

@velkymx velkymx left a comment

Choose a reason for hiding this comment

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

If you use a variable once, just remove it and put in the value.

$data['PRO_UID'] = $process->PRO_UID;
$data['PRO_ID'] = $process->PRO_ID;
$data['uid'] = Uuid::uuid4();
// $data['PRO_UID'] = $process->uid;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this

@@ -35,15 +23,34 @@ public function testCreateProcess(): Process
public function testCreateUser(): User
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not the correct place for this test.

* @depends testCreateUser
* @return Process
*/
public function testCreateProcess(User $user): Process
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not the right place. This should be in a process test

$this->auth($user->USR_USERNAME, self::DEFAULT_PASS);
$this->auth($user->username, self::DEFAULT_PASS);

$structure = [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just move this into $response->assertJsonStructure($structure); you're only using it once.

$url = self::API_ROUTE . $process->uid . '/dynaform';
$response = $this->api('POST', $url, $data);
//validating the answer is an error
$response->assertStatus(404);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is 404 the correct response? Page not found?

$id = $response->json('DYN_ID');
$id = $response->json('id');
//Check structure of response.
$response->assertJsonStructure($structure);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Put in JSON direct. No variable needed

@@ -270,8 +272,8 @@ public function testGetDynaform(Process $process, User $user, Dynaform $Dynaform
$response->assertJsonStructure($structure);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Insert json remove variable

//validating the answer is correct.
$response->assertStatus(422);
return Dynaform::where('DYN_ID', $id)->get()->first();
return Dynaform::where('id', $id)->get()->first();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

->get()->first() is redundant just use ->first()

@@ -270,8 +272,8 @@ public function testGetDynaform(Process $process, User $user, Dynaform $Dynaform
$response->assertJsonStructure($structure);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just use the JSON direct

$url = self::API_ROUTE . $process->uid . '/dynaform/' . $dynaform->uid;
$response = $this->api('DELETE', $url);
//Validate the answer is correct
$response->assertStatus(200);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use a different return on delete? A successful response SHOULD be 200 (OK) if the response includes an entity describing the status, 202 (Accepted) if the action has not yet been enacted, or 204 (No Content) if the action has been enacted but the response does not include an entity.

@velkymx velkymx added this to the 2018-05-17 Sprint milestone May 11, 2018
@velkymx velkymx merged commit 84d31e0 into develop May 11, 2018
@velkymx velkymx deleted the bugfix/124 branch August 10, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants