Skip to content

Commit da424bd

Browse files
committed
Clean up review findings
- Name the sentinel order in CardOcsController instead of a magic 999 - Drop unreachable stackAddCardAtTop case from ConfigService::get() - Restore the stronger visibility assertion in the label e2e test Signed-off-by: Theo <36564257+theoholl@users.noreply.github.com>
1 parent 5cd6419 commit da424bd

3 files changed

Lines changed: 10 additions & 5 deletions

File tree

cypress/e2e/cardFeatures.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ describe('Card', function () {
395395
cy.get(`.card:contains("${newCardTitle}")`)
396396
.scrollIntoView({ block: 'center' })
397397
.find('.labels li:contains("Later")')
398-
.should('exist')
398+
.should('be.visible')
399399
cy.get(`.card:contains("${newCardTitle}")`).find('.labels li:contains("Action needed")')
400400
.should('not.exist')
401401
})

lib/Controller/CardOcsController.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
use OCP\IRequest;
2222

2323
class CardOcsController extends OCSController {
24+
/**
25+
* Sentinel order used when the client does not request a specific position.
26+
* Cards created with this order are appended to the end of the stack.
27+
*/
28+
private const DEFAULT_ORDER = 999;
29+
2430
public function __construct(
2531
string $appName,
2632
IRequest $request,
@@ -36,7 +42,7 @@ public function __construct(
3642

3743
#[NoAdminRequired]
3844
#[PublicPage]
39-
public function create(string $title, int $stackId, ?int $boardId = null, ?string $type = 'plain', ?string $owner = null, ?int $order = 999, ?string $description = '', $duedate = null, $startdate = null, ?array $labels = [], ?array $users = [], ?string $color = null) {
45+
public function create(string $title, int $stackId, ?int $boardId = null, ?string $type = 'plain', ?string $owner = null, ?int $order = self::DEFAULT_ORDER, ?string $description = '', $duedate = null, $startdate = null, ?array $labels = [], ?array $users = [], ?string $color = null) {
4046
if ($boardId) {
4147
$board = $this->boardService->find($boardId, false);
4248
if ($board->getExternalId()) {
@@ -48,7 +54,8 @@ public function create(string $title, int $stackId, ?int $boardId = null, ?strin
4854
if (!$owner) {
4955
$owner = $this->userId;
5056
}
51-
$card = $this->cardService->create($title, $stackId, $type, $order, $owner, $description, $duedate, $startdate, $color, insertAtPosition: $order !== 999);
57+
// An explicit order means the client wants the card at that position, so shift the surrounding cards
58+
$card = $this->cardService->create($title, $stackId, $type, $order, $owner, $description, $duedate, $startdate, $color, insertAtPosition: $order !== self::DEFAULT_ORDER);
5259

5360
// foreach ($labels as $label) {
5461
// $this->assignLabel($card->getId(), $label);

lib/Service/ConfigService.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,6 @@ public function get(string $key) {
9191
return false;
9292
}
9393
return (bool)$this->config->getUserValue($this->getUserId(), Application::APP_ID, 'cardIdBadge', false);
94-
case 'stackAddCardAtTop':
95-
return $this->isStackAddCardAtTopEnabled();
9694
}
9795
return false;
9896
}

0 commit comments

Comments
 (0)