From 47809b97d3d81c6e359a75b5d210d51e91831605 Mon Sep 17 00:00:00 2001 From: poyuk Date: Wed, 18 Jan 2023 20:22:29 +0100 Subject: [PATCH 1/6] feat: filter null acs action values --- composer.json | 2 +- src/Model/AcsControl.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index e38a1da..90803f7 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "library", "license": "GPL-2.0-only", "require": { - "php": ">=7.2.0", + "php": ">=7.4.0", "ext-json": "*", "oat-sa/lib-lti1p3-core": "^6.2" }, diff --git a/src/Model/AcsControl.php b/src/Model/AcsControl.php index 1a636d4..b55da21 100644 --- a/src/Model/AcsControl.php +++ b/src/Model/AcsControl.php @@ -237,7 +237,8 @@ public function jsonSerialize(): array 'incident_severity' => $this->incidentSeverity, 'reason_code' => $this->reasonCode, 'reason_msg' => $this->reasonMessage, - ] + ], + static fn($element) => $element !== null ); } } From 6f29fdca0fad5763b52d50c18b47bf3cbcc42037 Mon Sep 17 00:00:00 2001 From: poyuk Date: Fri, 20 Jan 2023 09:41:04 +0100 Subject: [PATCH 2/6] fix: add support of 0 extra time in acs control factory --- src/Factory/AcsControlFactory.php | 8 +- src/Factory/AcsControlResultFactory.php | 4 +- src/Model/AcsControlResult.php | 3 +- tests/Unit/Factory/AcsControlFactoryTest.php | 80 ++++++++++++++++++++ 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/Factory/AcsControlFactory.php b/src/Factory/AcsControlFactory.php index db98e7a..b6f5c1a 100644 --- a/src/Factory/AcsControlFactory.php +++ b/src/Factory/AcsControlFactory.php @@ -94,8 +94,8 @@ public function create(array $data): AcsControlInterface throw new InvalidArgumentException('Missing mandatory incident_time'); } - $extraTime = $data['extra_time'] ?? null; - $incidentSeverity = $data['incident_severity'] ?? null; + $extraTime = isset($data['extra_time']) ? (int)$data['extra_time'] : null; + $incidentSeverity = isset($data['incident_severity']) ? (float)$data['incident_severity'] : null; return new AcsControl( $resourceLink, @@ -104,8 +104,8 @@ public function create(array $data): AcsControlInterface new Carbon($incidentTime), $attemptNumber, $issuerIdentifier, - $extraTime ? intval($extraTime) : null, - $incidentSeverity ? floatval($incidentSeverity) : null, + $extraTime, + $incidentSeverity, $data['reason_code'] ?? null, $data['reason_msg'] ?? null ); diff --git a/src/Factory/AcsControlResultFactory.php b/src/Factory/AcsControlResultFactory.php index 6169243..e5270fe 100644 --- a/src/Factory/AcsControlResultFactory.php +++ b/src/Factory/AcsControlResultFactory.php @@ -40,8 +40,8 @@ public function create(array $data): AcsControlResultInterface throw new LtiException('Cannot create ACS control result: Invalid status'); } - $extraTime = $data['extra_time'] ?? null; + $extraTime = isset($data['extra_time']) ? (int)$data['extra_time'] : null; - return new AcsControlResult($status, $extraTime ? intval($extraTime) : null); + return new AcsControlResult($status, $extraTime); } } diff --git a/src/Model/AcsControlResult.php b/src/Model/AcsControlResult.php index d7b569b..4db5db4 100644 --- a/src/Model/AcsControlResult.php +++ b/src/Model/AcsControlResult.php @@ -79,7 +79,8 @@ public function jsonSerialize(): array [ 'status' => $this->status, 'extra_time' => $this->extraTime, - ] + ], + static fn($element) => $element !== null ); } } diff --git a/tests/Unit/Factory/AcsControlFactoryTest.php b/tests/Unit/Factory/AcsControlFactoryTest.php index fa6dd67..e651430 100644 --- a/tests/Unit/Factory/AcsControlFactoryTest.php +++ b/tests/Unit/Factory/AcsControlFactoryTest.php @@ -80,6 +80,86 @@ public function testCreateSuccess(): void $this->assertEquals('message', $result->getReasonMessage()); } + public function testCreateSuccessWithZeroExtraTimeAndIncidentSeverity(): void + { + $date = Carbon::now(); + + $result = $this->subject->create( + [ + 'user' => [ + 'iss' => 'issuerIdentifier', + 'sub' => 'userIdentifier', + ], + 'resource_link' => [ + 'id' => 'resourceLinkIdentifier', + 'title' => 'resourceLinkTitle', + 'description' => 'resourceLinkDescription', + ], + 'attempt_number' => 1, + 'action' => AcsControlInterface::ACTION_UPDATE, + 'extra_time' => 0, + 'incident_time' => $date->format(DATE_ATOM), + 'incident_severity' => 0, + 'reason_code' => 'code', + 'reason_msg' => 'message', + ] + ); + + $this->assertInstanceOf(AcsControlInterface::class, $result); + + $this->assertEquals('resourceLinkIdentifier', $result->getResourceLink()->getIdentifier()); + $this->assertEquals('resourceLinkTitle', $result->getResourceLink()->getTitle()); + $this->assertEquals('resourceLinkDescription', $result->getResourceLink()->getText()); + $this->assertEquals('issuerIdentifier', $result->getIssuerIdentifier()); + $this->assertEquals('userIdentifier', $result->getUserIdentifier()); + $this->assertEquals(1, $result->getAttemptNumber()); + $this->assertEquals(AcsControlInterface::ACTION_UPDATE, $result->getAction()); + $this->assertEquals(0, $result->getExtraTime()); + $this->assertEquals($date->format(DATE_ATOM), $result->getIncidentTime()->format(DATE_ATOM)); + $this->assertEquals(0, $result->getIncidentSeverity()); + $this->assertEquals('code', $result->getReasonCode()); + $this->assertEquals('message', $result->getReasonMessage()); + } + + public function testCreateSuccessWithoutExtraTimeAndIncidentSeverity(): void + { + $date = Carbon::now(); + + $result = $this->subject->create( + [ + 'user' => [ + 'iss' => 'issuerIdentifier', + 'sub' => 'userIdentifier', + ], + 'resource_link' => [ + 'id' => 'resourceLinkIdentifier', + 'title' => 'resourceLinkTitle', + 'description' => 'resourceLinkDescription', + ], + 'attempt_number' => 1, + 'action' => AcsControlInterface::ACTION_UPDATE, + 'incident_time' => $date->format(DATE_ATOM), + 'reason_code' => 'code', + 'reason_msg' => 'message', + ] + ); + + $this->assertInstanceOf(AcsControlInterface::class, $result); + + $this->assertEquals('resourceLinkIdentifier', $result->getResourceLink()->getIdentifier()); + $this->assertEquals('resourceLinkTitle', $result->getResourceLink()->getTitle()); + $this->assertEquals('resourceLinkDescription', $result->getResourceLink()->getText()); + $this->assertEquals('issuerIdentifier', $result->getIssuerIdentifier()); + $this->assertEquals('userIdentifier', $result->getUserIdentifier()); + $this->assertEquals(1, $result->getAttemptNumber()); + $this->assertEquals(AcsControlInterface::ACTION_UPDATE, $result->getAction()); + $this->assertEquals(null, $result->getExtraTime()); + $this->assertEquals($date->format(DATE_ATOM), $result->getIncidentTime()->format(DATE_ATOM)); + $this->assertEquals(null, $result->getIncidentSeverity()); + $this->assertEquals('code', $result->getReasonCode()); + $this->assertEquals('message', $result->getReasonMessage()); + } + /** * @dataProvider provideFailingData */ From c5595992743a4db2ae6569c4544daefad89a5e86 Mon Sep 17 00:00:00 2001 From: poyuk Date: Fri, 20 Jan 2023 09:51:41 +0100 Subject: [PATCH 3/6] chore!: increase supported php level on 7.4 --- .github/workflows/build.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 28c7876..7ce57d1 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - php: [7.2, 7.3, 7.4] + php: [7.4] coverage: ["true"] include: - php: 8.0 @@ -39,7 +39,7 @@ jobs: - name: Psalm run: | ./vendor/bin/psalm --shepherd - + - name: Coveralls if: ${{ matrix.coverage == 'true' }} env: From 9c11e849cd1626787180d853b9dd5e546b720a06 Mon Sep 17 00:00:00 2001 From: poyuk Date: Fri, 20 Jan 2023 12:18:18 +0100 Subject: [PATCH 4/6] chore: code style adjustment --- src/Factory/AcsControlFactory.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Factory/AcsControlFactory.php b/src/Factory/AcsControlFactory.php index b6f5c1a..c08dfcb 100644 --- a/src/Factory/AcsControlFactory.php +++ b/src/Factory/AcsControlFactory.php @@ -56,7 +56,8 @@ public function create(array $data): AcsControlInterface [ 'title' => $resourceLinkData['title'] ?? null, 'text' => $resourceLinkData['description'] ?? null, - ]); + ] + ); $userData = $data['user'] ?? null; From df7ad0bf12c1650ed7c0d1a372a2cdedb2674905 Mon Sep 17 00:00:00 2001 From: poyuk Date: Mon, 23 Jan 2023 18:18:31 +0100 Subject: [PATCH 5/6] chore: rollback to php 7.2 compatibility --- .github/workflows/build.yaml | 4 ++-- composer.json | 2 +- src/Model/AcsControl.php | 4 +++- src/Model/AcsControlResult.php | 4 +++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 7ce57d1..e102d38 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -9,10 +9,10 @@ jobs: strategy: fail-fast: false matrix: - php: [7.4] + php: [7.2, 7.3, 7.4, 8.0, 8.1, 8.2] coverage: ["true"] include: - - php: 8.0 + - php: 8.2 coverage: "false" # PHPUnit 8.5.14 doesn't support code coverage under PHP 8 steps: diff --git a/composer.json b/composer.json index 90803f7..e38a1da 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "library", "license": "GPL-2.0-only", "require": { - "php": ">=7.4.0", + "php": ">=7.2.0", "ext-json": "*", "oat-sa/lib-lti1p3-core": "^6.2" }, diff --git a/src/Model/AcsControl.php b/src/Model/AcsControl.php index b55da21..6ba7c44 100644 --- a/src/Model/AcsControl.php +++ b/src/Model/AcsControl.php @@ -238,7 +238,9 @@ public function jsonSerialize(): array 'reason_code' => $this->reasonCode, 'reason_msg' => $this->reasonMessage, ], - static fn($element) => $element !== null + static function ($element) { + return $element !== null; + } ); } } diff --git a/src/Model/AcsControlResult.php b/src/Model/AcsControlResult.php index 4db5db4..e861128 100644 --- a/src/Model/AcsControlResult.php +++ b/src/Model/AcsControlResult.php @@ -80,7 +80,9 @@ public function jsonSerialize(): array 'status' => $this->status, 'extra_time' => $this->extraTime, ], - static fn($element) => $element !== null + static function($element) { + return $element !== null; + } ); } } From 209cadb41aa3e0de638af0744d7ef74945143ba0 Mon Sep 17 00:00:00 2001 From: poyuk Date: Tue, 24 Jan 2023 10:27:20 +0100 Subject: [PATCH 6/6] chore: ignore php >=8 for coverage --- .github/workflows/build.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index e102d38..c709a01 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -9,9 +9,13 @@ jobs: strategy: fail-fast: false matrix: - php: [7.2, 7.3, 7.4, 8.0, 8.1, 8.2] + php: [7.2, 7.3, 7.4] coverage: ["true"] include: + - php: 8.0 + coverage: "false" # PHPUnit 8.5.14 doesn't support code coverage under PHP 8 + - php: 8.1 + coverage: "false" # PHPUnit 8.5.14 doesn't support code coverage under PHP 8 - php: 8.2 coverage: "false" # PHPUnit 8.5.14 doesn't support code coverage under PHP 8