Only allow workflows on owned boards, and show the board in the table
Build package / php-lint (8.4) (push) Successful in 1m7s
Build package / php-lint (8.5) (push) Successful in 56s
Build package / xml-lint (push) Successful in 17s
Build package / unit-tests (push) Successful in 42s
Build package / package (push) Successful in 1m4s
Build package / php-lint (8.4) (push) Successful in 1m7s
Build package / php-lint (8.5) (push) Successful in 56s
Build package / xml-lint (push) Successful in 17s
Build package / unit-tests (push) Successful in 42s
Build package / package (push) Successful in 1m4s
The board list now only contains boards the current user owns, which also makes the controller's board validation enforce the owner rule. The workflow table gains a Board column (Board, Titel, Von Stapel, Nach Stapel, Filter, E-Mail, Aktiv) and is sorted alphabetically by those columns in that order.
This commit is contained in:
1 parent
7c7c52cb1c
commit
ada16cb101
4 files changed
+85
-22
No files matched your search
@@ -40,6 +40,12 @@ The filter logic (`WorkflowRunner::cardMatchesFilters()`, `::isOverdue()`, `::fi
|
||||
|
||||
Archived boards deliberately do *not* trigger this (they stay readable, so `findStackIds()` succeeds), even though they're filtered out of the settings dropdowns.
|
||||
|
||||
## Workflows only on boards the user owns
|
||||
|
||||
A workflow can only be created or saved on a board whose Deck owner is the current user. `DeckIntegrationService::listBoardsForCurrentUser($uid)` returns **only owned boards** (`unwrapUid($board->getOwner()) === $uid`); boards merely shared with the user are dropped. Since `WorkflowController::validate()` checks the board id against that list, the same filter enforces the rule server-side — there is no separate ownership check.
|
||||
|
||||
Workflows created before this rule may still sit on a shared board. The runner does not check ownership, so they keep running; in the settings table their board is missing from the list, so the row is marked "Board nicht mehr vorhanden oder nicht dein eigenes" and saving it requires picking an owned board.
|
||||
|
||||
## Dead filters are surfaced, never disabled
|
||||
|
||||
`findBrokenTarget()` covers the board and the two stacks — **not** `filter_user_ids` / `filter_label_ids`. Those are plain id lists compared with `array_intersect()` in `cardMatchesFilters()`, so an entry that no longer exists simply stops matching. With three labels of which one was deleted that is exactly right, and disabling would be wrong.
|
||||
|
||||
@@ -85,14 +85,14 @@ class WorkflowController extends OCSController {
|
||||
bool $notifyEmail = false,
|
||||
bool $enabled = true,
|
||||
): DataResponse {
|
||||
$this->validate($title, $boardId, $sourceStackId, $targetStackId);
|
||||
|
||||
try {
|
||||
$workflow = $this->workflowMapper->findForUser($id, $this->currentUserId());
|
||||
} catch (DoesNotExistException $e) {
|
||||
throw new OCSNotFoundException('Workflow not found');
|
||||
}
|
||||
|
||||
$this->validate($title, $boardId, $sourceStackId, $targetStackId);
|
||||
|
||||
$this->applyFields($workflow, $title, $boardId, $sourceStackId, $targetStackId, $filterUserIds, $filterLabelIds, $notifyEmail, $enabled);
|
||||
$workflow = $this->workflowMapper->update($workflow);
|
||||
|
||||
@@ -114,7 +114,7 @@ class WorkflowController extends OCSController {
|
||||
#[NoAdminRequired]
|
||||
public function boards(): DataResponse {
|
||||
$this->assertDeck();
|
||||
return new DataResponse($this->deckService->listBoardsForCurrentUser());
|
||||
return new DataResponse($this->deckService->listBoardsForCurrentUser($this->currentUserId()));
|
||||
}
|
||||
|
||||
#[NoAdminRequired]
|
||||
@@ -161,12 +161,14 @@ class WorkflowController extends OCSController {
|
||||
// not "Deck is unreachable".
|
||||
$this->assertDeck();
|
||||
|
||||
// Only boards the user owns are listed, so this also enforces the
|
||||
// owner rule.
|
||||
$boardIds = array_map(
|
||||
static fn (array $board) => (int)$board['id'],
|
||||
$this->deckService->listBoardsForCurrentUser(),
|
||||
$this->deckService->listBoardsForCurrentUser($this->currentUserId()),
|
||||
);
|
||||
if (!in_array($boardId, $boardIds, true)) {
|
||||
throw new OCSBadRequestException('The selected board does not exist or is not accessible');
|
||||
throw new OCSBadRequestException('The selected board does not exist or is not owned by you');
|
||||
}
|
||||
|
||||
$stackIds = array_map(
|
||||
|
||||
@@ -174,13 +174,16 @@ class DeckIntegrationService {
|
||||
/**
|
||||
* @return array<int, array{id: int, title: string}>
|
||||
*
|
||||
* Only boards owned by `$userId` — workflows may only be created on those,
|
||||
* so boards merely shared with the user are left out entirely.
|
||||
*
|
||||
* Only ever call this from a *request*: getUserBoards() reads the uid
|
||||
* BoardService froze at construction time, and beginUserContext() cannot
|
||||
* help here — it repoints that field, but Deck may already have cached
|
||||
* board lists behind it, and the job has no use for a board *list*
|
||||
* anyway. It asks findStackIds() about one known board id instead.
|
||||
*/
|
||||
public function listBoardsForCurrentUser(): array {
|
||||
public function listBoardsForCurrentUser(string $userId): array {
|
||||
$boards = $this->call(function () {
|
||||
$boardService = $this->resolve(BoardService::class);
|
||||
try {
|
||||
@@ -198,12 +201,13 @@ class DeckIntegrationService {
|
||||
}, []);
|
||||
|
||||
// Deck merges own/group/circle boards, so the same board can come
|
||||
// back more than once; and boards in the trash or archived ones are
|
||||
// no useful automation target. Keyed by id => deduplicated.
|
||||
// back more than once; boards in the trash or archived ones are no
|
||||
// useful automation target, and shared ones are not allowed. Keyed
|
||||
// by id => deduplicated.
|
||||
$result = [];
|
||||
foreach ($boards as $board) {
|
||||
$id = (int)$board->getId();
|
||||
if (isset($result[$id]) || $this->isBoardHidden($board)) {
|
||||
if (isset($result[$id]) || $this->isBoardHidden($board) || $this->unwrapUid($board->getOwner()) !== $userId) {
|
||||
continue;
|
||||
}
|
||||
$title = $board->getTitle();
|
||||
|
||||
+64
-13
@@ -14,6 +14,7 @@
|
||||
<table v-if="workflows.length" class="wfda-table">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Board</th>
|
||||
<th>Titel</th>
|
||||
<th>Von Stapel</th>
|
||||
<th>Nach Stapel</th>
|
||||
@@ -24,7 +25,8 @@
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
<tr v-for="workflow in workflows" :key="workflow.id">
|
||||
<tr v-for="workflow in sortedWorkflows" :key="workflow.id">
|
||||
<td>{{ boardLabel(workflow.boardId) }}</td>
|
||||
<td>
|
||||
{{ workflow.title }}
|
||||
<span v-if="workflowIssues[workflow.id]" class="wfda-issue">
|
||||
@@ -33,16 +35,9 @@
|
||||
</td>
|
||||
<td>{{ stackLabel(workflow.boardId, workflow.sourceStackId) }}</td>
|
||||
<td>{{ stackLabel(workflow.boardId, workflow.targetStackId) }}</td>
|
||||
<td>
|
||||
<span v-if="!workflow.filterUserIds.length && !workflow.filterLabelIds.length">–</span>
|
||||
<span v-else>
|
||||
<span v-if="workflow.filterUserIds.length">{{ workflow.filterUserIds.length }} Benutzer</span>
|
||||
<span v-if="workflow.filterUserIds.length && workflow.filterLabelIds.length">, </span>
|
||||
<span v-if="workflow.filterLabelIds.length">{{ workflow.filterLabelIds.length }} Label</span>
|
||||
</span>
|
||||
</td>
|
||||
<td>{{ workflow.notifyEmail ? 'Ja' : 'Nein' }}</td>
|
||||
<td>{{ workflow.enabled ? 'Ja' : 'Nein' }}</td>
|
||||
<td>{{ filterLabel(workflow) }}</td>
|
||||
<td>{{ yesNo(workflow.notifyEmail) }}</td>
|
||||
<td>{{ yesNo(workflow.enabled) }}</td>
|
||||
<td class="wfda-actions">
|
||||
<NcButton variant="tertiary" @click="editWorkflow(workflow)">
|
||||
Bearbeiten
|
||||
@@ -75,6 +70,9 @@
|
||||
:reduce="option => option.value"
|
||||
placeholder="Board wählen"
|
||||
@update:model-value="onBoardChange" />
|
||||
<NcNoteCard v-if="!boardOptions.length" type="info">
|
||||
Workflows können nur für Boards angelegt werden, deren Eigentümer du bist.
|
||||
</NcNoteCard>
|
||||
|
||||
<NcSelect
|
||||
v-model="form.sourceStackId"
|
||||
@@ -175,7 +173,40 @@ const participantOptions = ref([])
|
||||
// Which board the three lists above currently belong to.
|
||||
const loadedBoardId = ref(null)
|
||||
|
||||
const boardOptions = computed(() => boards.value.map((board) => ({ value: board.id, label: board.title })))
|
||||
const collator = new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' })
|
||||
|
||||
// The backend only returns boards the user owns.
|
||||
const boardOptions = computed(() => boards.value
|
||||
.map((board) => ({ value: board.id, label: board.title }))
|
||||
.sort((a, b) => collator.compare(a.label, b.label)))
|
||||
|
||||
/**
|
||||
* Alphabetical by the columns as displayed, left to right: board, title,
|
||||
* source stack, target stack, filter, e-mail, active.
|
||||
*/
|
||||
const sortedWorkflows = computed(() => {
|
||||
const sortKeys = (workflow) => [
|
||||
boardLabel(workflow.boardId),
|
||||
workflow.title,
|
||||
stackLabel(workflow.boardId, workflow.sourceStackId),
|
||||
stackLabel(workflow.boardId, workflow.targetStackId),
|
||||
filterLabel(workflow),
|
||||
yesNo(workflow.notifyEmail),
|
||||
yesNo(workflow.enabled),
|
||||
]
|
||||
return workflows.value
|
||||
.map((workflow) => ({ workflow, keys: sortKeys(workflow) }))
|
||||
.sort((a, b) => {
|
||||
for (let i = 0; i < a.keys.length; i++) {
|
||||
const result = collator.compare(a.keys[i], b.keys[i])
|
||||
if (result !== 0) {
|
||||
return result
|
||||
}
|
||||
}
|
||||
return a.workflow.id - b.workflow.id
|
||||
})
|
||||
.map((entry) => entry.workflow)
|
||||
})
|
||||
|
||||
// Source and target must differ, so each dropdown hides what the other one
|
||||
// already holds — the error message on save stays as the backstop for
|
||||
@@ -199,7 +230,7 @@ const workflowIssues = computed(() => {
|
||||
|
||||
for (const workflow of workflows.value) {
|
||||
if (!boards.value.some((board) => board.id === workflow.boardId)) {
|
||||
issues[workflow.id] = 'Board nicht mehr vorhanden'
|
||||
issues[workflow.id] = 'Board nicht mehr vorhanden oder nicht dein eigenes'
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -262,6 +293,26 @@ function emptyForm() {
|
||||
}
|
||||
}
|
||||
|
||||
function boardLabel(boardId) {
|
||||
const board = boards.value.find((entry) => entry.id === boardId)
|
||||
return board ? board.title : `#${boardId}`
|
||||
}
|
||||
|
||||
function filterLabel(workflow) {
|
||||
const parts = []
|
||||
if (workflow.filterUserIds.length) {
|
||||
parts.push(`${workflow.filterUserIds.length} Benutzer`)
|
||||
}
|
||||
if (workflow.filterLabelIds.length) {
|
||||
parts.push(`${workflow.filterLabelIds.length} Label`)
|
||||
}
|
||||
return parts.length ? parts.join(', ') : '–'
|
||||
}
|
||||
|
||||
function yesNo(value) {
|
||||
return value ? 'Ja' : 'Nein'
|
||||
}
|
||||
|
||||
function stackLabel(boardId, stackId) {
|
||||
const stacks = stacksByBoard[boardId]
|
||||
if (!stacks) {
|
||||
|
||||
Reference in new issue
Block a user