Skip to content

Commit 0f62241

Browse files
KocKoc
committed
Enhancement: Add validation for unique values for short text fields (code review 2)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent e2c7b5f commit 0f62241

File tree

8 files changed

+140
-11
lines changed

8 files changed

+140
-11
lines changed

‎lib/Controller/Api1Controller.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,9 +1139,10 @@ public function indexViewRows(int $viewId, ?int $limit, ?int $offset): DataRespo
11391139
*
11401140
* @param int $viewId View ID
11411141
* @param string|array<string, mixed> $data Data as key - value store
1142-
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
1142+
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
11431143
*
11441144
* 200: Row returned
1145+
* 400: Validation error
11451146
* 403: No permissions
11461147
*/
11471148
#[NoAdminRequired]
@@ -1168,11 +1169,14 @@ public function createRowInView(int $viewId, $data): DataResponse {
11681169

11691170
try {
11701171
return new DataResponse($this->rowService->create(null, $viewId, $dataNew)->jsonSerialize());
1172+
} catch (BadRequestError $e) {
1173+
$this->logger->warning('An bad request was encountered: ' . $e->getMessage(), ['exception' => $e]);
1174+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
11711175
} catch (PermissionError $e) {
11721176
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
11731177
$message = ['message' => $e->getMessage()];
11741178
return new DataResponse($message, Http::STATUS_FORBIDDEN);
1175-
} catch (BadRequestError|InternalError|Exception $e) {
1179+
} catch (InternalError|Exception $e) {
11761180
$this->logger->error('An internal error or exception occurred: ' . $e->getMessage(), ['exception' => $e]);
11771181
$message = ['message' => $e->getMessage()];
11781182
return new DataResponse($message, Http::STATUS_INTERNAL_SERVER_ERROR);
@@ -1184,9 +1188,10 @@ public function createRowInView(int $viewId, $data): DataResponse {
11841188
*
11851189
* @param int $tableId Table ID
11861190
* @param string|array<string, mixed> $data Data as key - value store
1187-
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
1191+
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
11881192
*
11891193
* 200: Row returned
1194+
* 400: Validation error
11901195
* 403: No permissions
11911196
* 404: Not found
11921197
*/
@@ -1214,11 +1219,14 @@ public function createRowInTable(int $tableId, $data): DataResponse {
12141219

12151220
try {
12161221
return new DataResponse($this->rowService->create($tableId, null, $dataNew)->jsonSerialize());
1222+
} catch (BadRequestError $e) {
1223+
$this->logger->warning('An bad request was encountered: ' . $e->getMessage(), ['exception' => $e]);
1224+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
12171225
} catch (PermissionError $e) {
12181226
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
12191227
$message = ['message' => $e->getMessage()];
12201228
return new DataResponse($message, Http::STATUS_FORBIDDEN);
1221-
} catch (BadRequestError|InternalError|Exception $e) {
1229+
} catch (InternalError|Exception $e) {
12221230
$this->logger->error('An internal error or exception occurred: ' . $e->getMessage(), ['exception' => $e]);
12231231
$message = ['message' => $e->getMessage()];
12241232
return new DataResponse($message, Http::STATUS_INTERNAL_SERVER_ERROR);
@@ -1263,9 +1271,10 @@ public function getRow(int $rowId): DataResponse {
12631271
* @param int|null $viewId View ID
12641272
* @param string|array<string, mixed> $data Data as key - value store
12651273
*
1266-
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
1274+
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
12671275
*
12681276
* 200: Updated row returned
1277+
* 400: Validation error
12691278
* 403: No permissions
12701279
* 404: Not found
12711280
*/
@@ -1291,6 +1300,9 @@ public function updateRow(int $rowId, ?int $viewId, $data): DataResponse {
12911300

12921301
try {
12931302
return new DataResponse($this->rowService->updateSet($rowId, $viewId, $dataNew, $this->userId)->jsonSerialize());
1303+
} catch (BadRequestError $e) {
1304+
$this->logger->warning('An bad request was encountered: ' . $e->getMessage(), ['exception' => $e]);
1305+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
12941306
} catch (InternalError $e) {
12951307
$this->logger->error('An internal error or exception occurred: ' . $e->getMessage(), ['exception' => $e]);
12961308
$message = ['message' => $e->getMessage()];

‎lib/Controller/Errors.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use Closure;
1111
use InvalidArgumentException;
12+
use OCA\Tables\Errors\BadRequestError;
1213
use OCA\Tables\Errors\InternalError;
1314
use OCA\Tables\Errors\NotFoundError;
1415
use OCA\Tables\Errors\PermissionError;
@@ -20,6 +21,9 @@ trait Errors {
2021
protected function handleError(Closure $callback): DataResponse {
2122
try {
2223
return new DataResponse($callback());
24+
} catch (BadRequestError $e) {
25+
$this->logger->warning('An bad request was encountered: ' . $e->getMessage(), ['exception' => $e]);
26+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
2327
} catch (PermissionError $e) {
2428
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
2529
$message = ['message' => $e->getMessage()];

‎lib/Service/RowService.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ private function getRowById(int $rowId): Row2 {
386386
* @param string $userId
387387
* @return Row2
388388
*
389+
* @throws BadRequestError
389390
* @throws InternalError
390391
* @throws NotFoundError
391392
* @throws PermissionError

‎openapi.json

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4148,6 +4148,24 @@
41484148
}
41494149
}
41504150
},
4151+
"400": {
4152+
"description": "Validation error",
4153+
"content": {
4154+
"application/json": {
4155+
"schema": {
4156+
"type": "object",
4157+
"required": [
4158+
"message"
4159+
],
4160+
"properties": {
4161+
"message": {
4162+
"type": "string"
4163+
}
4164+
}
4165+
}
4166+
}
4167+
}
4168+
},
41514169
"500": {
41524170
"description": "",
41534171
"content": {
@@ -4364,6 +4382,24 @@
43644382
}
43654383
}
43664384
},
4385+
"400": {
4386+
"description": "Validation error",
4387+
"content": {
4388+
"application/json": {
4389+
"schema": {
4390+
"type": "object",
4391+
"required": [
4392+
"message"
4393+
],
4394+
"properties": {
4395+
"message": {
4396+
"type": "string"
4397+
}
4398+
}
4399+
}
4400+
}
4401+
}
4402+
},
43674403
"500": {
43684404
"description": "",
43694405
"content": {
@@ -4563,6 +4599,24 @@
45634599
}
45644600
}
45654601
},
4602+
"400": {
4603+
"description": "Validation error",
4604+
"content": {
4605+
"application/json": {
4606+
"schema": {
4607+
"type": "object",
4608+
"required": [
4609+
"message"
4610+
],
4611+
"properties": {
4612+
"message": {
4613+
"type": "string"
4614+
}
4615+
}
4616+
}
4617+
}
4618+
}
4619+
},
45664620
"500": {
45674621
"description": "",
45684622
"content": {

‎src/modules/modals/EditRow.vue

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,12 @@ export default {
151151
},
152152
async actionConfirm() {
153153
this.localLoading = true
154-
await this.sendRowToBE()
154+
const success = await this.sendRowToBE()
155155
this.localLoading = false
156+
// If the row was not created, we don't want to close the modal
157+
if (!success) {
158+
return
159+
}
156160
this.actionCancel()
157161
},
158162
async sendRowToBE() {
@@ -166,15 +170,12 @@ export default {
166170
})
167171
}
168172

169-
const res = await this.updateRow({
173+
return await this.updateRow({
170174
id: this.row.id,
171175
isView: this.isView,
172176
elementId: this.element.id,
173177
data,
174178
})
175-
if (!res) {
176-
showError(t('tables', 'Could not update row'))
177-
}
178179
},
179180
reset() {
180181
this.localRow = {}

‎src/shared/utils/displayError.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ function statusMessage(e, status) {
2424
if (e.response?.data?.ocs?.data?.message) {
2525
return e.response.data.ocs.data.message
2626
}
27+
// for some reason the "edit" row returns a different structure
28+
if (e.response?.data?.message) {
29+
return e.response.data.message
30+
}
2731
return t('tables', 'Unknown error.')
2832
}
2933
if (status === 401) {

‎src/types/openapi/openapi.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2840,6 +2840,17 @@ export interface operations {
28402840
readonly "application/json": components["schemas"]["Row"];
28412841
};
28422842
};
2843+
/** @description Validation error */
2844+
readonly 400: {
2845+
headers: {
2846+
readonly [name: string]: unknown;
2847+
};
2848+
content: {
2849+
readonly "application/json": {
2850+
readonly message: string;
2851+
};
2852+
};
2853+
};
28432854
/** @description No permissions */
28442855
readonly 403: {
28452856
headers: {
@@ -2953,6 +2964,17 @@ export interface operations {
29532964
readonly "application/json": components["schemas"]["Row"];
29542965
};
29552966
};
2967+
/** @description Validation error */
2968+
readonly 400: {
2969+
headers: {
2970+
readonly [name: string]: unknown;
2971+
};
2972+
content: {
2973+
readonly "application/json": {
2974+
readonly message: string;
2975+
};
2976+
};
2977+
};
29562978
/** @description No permissions */
29572979
readonly 403: {
29582980
headers: {
@@ -3066,6 +3088,17 @@ export interface operations {
30663088
readonly "application/json": components["schemas"]["Row"];
30673089
};
30683090
};
3091+
/** @description Validation error */
3092+
readonly 400: {
3093+
headers: {
3094+
readonly [name: string]: unknown;
3095+
};
3096+
content: {
3097+
readonly "application/json": {
3098+
readonly message: string;
3099+
};
3100+
};
3101+
};
30693102
/** @description No permissions */
30703103
readonly 403: {
30713104
headers: {

‎tests/integration/features/APIv2.feature

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,7 @@ Feature: APIv2
14421442
Then "view" "v1" has exactly these rows "r1,r3,r2" in exactly this order
14431443

14441444
@api2 @rows
1445-
Scenario: Try to create rows with with non-unique values should fail
1445+
Scenario: Try to create row with with non-unique values should fail
14461446
Given table "Table with unique column" with emoji "👨🏻‍💻" exists for user "participant1-v2" as "base1" via v2
14471447
And column "one" exists with following properties
14481448
| type | text |
@@ -1456,3 +1456,23 @@ Feature: APIv2
14561456
When user "participant1-v2" tries to create a row using v2 with following values
14571457
| one | AHA |
14581458
Then the reported status is 400
1459+
1460+
@api2 @rows
1461+
Scenario: Try to edit row with with non-unique values should fail
1462+
Given table "Table with unique column" with emoji "👨🏻‍💻" exists for user "participant1-v2" as "base1" via v2
1463+
And column "one" exists with following properties
1464+
| type | text |
1465+
| subtype | line |
1466+
| mandatory | 1 |
1467+
| description | This is a description! |
1468+
| textUnique | true |
1469+
And using table "base1"
1470+
When user "participant1-v2" creates row "row-1" with following values:
1471+
| one | AHA |
1472+
Then the reported status is 200
1473+
When user "participant1-v2" creates row "row-2" with following values:
1474+
| one | YES |
1475+
Then the reported status is 200
1476+
When user "participant1-v2" updates row "row-1" with following values:
1477+
| one | YES |
1478+
Then the reported status is "400"

0 commit comments

Comments
 (0)