feat(workflowengine): add name, description and collapsible UI to flow rules#60981
Conversation
letmefixthiscode
requested review from
a team,
blizzz,
nickvergessen and
provokateurin
as code owners
letmefixthiscode
requested review from
ArtificialOwl,
artonge,
come-nc,
nfebe,
sorbaugh and
susnux
and removed request for
a team
artonge
left a comment
artonge
left a comment
There was a problem hiding this comment.
cc @nextcloud/designers to review whether this need changes.
and cc @salmart-dev for awareness.
|
|
||
| $table = $schema->getTable('flow_operations'); | ||
| if (!$table->hasColumn('description')) { | ||
| $table->addColumn('description', Types::TEXT, [ |
There was a problem hiding this comment.
As the description is limited in length, shouldn't it be a Types::STRING with a 'length' => 4000 like the name column?
There was a problem hiding this comment.
look here:
server/lib/private/DB/Migrator.php
Line 69 in 732c234
There was a problem hiding this comment.
Maybe @Altahrim has an idea of what's the best solution
There was a problem hiding this comment.
A long description sounds okay and I don't see why we would have to strictly limit it?
| :model-value="inputValue" | ||
| @update:model-value="updateOperationByEvent" /> |
| @input="updateOperation" /> | ||
| </Operation> | ||
| <div class="buttons"> | ||
| <NcButton v-if="rule.id < -1 || dirty" @click="cancelRule"> |
There was a problem hiding this comment.
for me its way more clear to check rule.id < 1 (because in the database we have 1..n) the code was not checking for 0 .. or -1 - also in the old code had rule.id < -1 and rule.id < 0 checks
| <component :is="expanded ? 'MenuUp' : 'MenuDown'" :size="20" /> | ||
| </span> | ||
| </button> | ||
| <div v-show="expanded" :id="bodyId" class="rule__body"> |
There was a problem hiding this comment.
Would the <details> HTML component reduce the custom logic and improve accessibility?
| nameMaxLength: NAME_MAX_LENGTH, | ||
| descriptionMaxLength: DESCRIPTION_MAX_LENGTH, |
There was a problem hiding this comment.
As it is used in only one location, we can skip creating global variables and use the value directly in the HTML template.
DB auto-increment starts at 1; new rules get a negative timestamp as temporary id (-(new Date().getTime())), so id < 0 was close but semantically wrong, and id < -1 in the Cancel button never matched a brand-new rule at all. Aligning all three guards (dirty init, Cancel visibility, cancelRule branch) plus the store's POST-vs-PUT decision to id < 1 makes the contract explicit: anything below 1 is a client-side draft. Signed-off-by: Dominic Blaß <letmefixthis.code@gmail.com>
…w rules - Add optional name (max 256 chars) and description (no limit) fields to flow rules - Collapse saved rules; auto-expand new unsaved rules using native HTML5 <details>/<summary> - Remove custom toggle logic and aria-expanded in favour of native details semantics - Drop MAX_DESCRIPTION_BYTES backend validation (description is now unbounded) - Add DB migration adding nullable TEXT description column to flow_operations Signed-off-by: Dominic Blaß <letmefixthis.code@gmail.com>
letmefixthiscode
force-pushed
the
feat/workflowengine-rule-name-description
branch
from
d8e01f8 to
1c4d728
Compare
4 participants
Summary
Checklist
3. to review, feature component)stable32)AI