Skip to content

Performance fix - #529

Merged
rubenvdlinde merged 1 commit into
developmentfrom
hotfix/settingspage
Aug 13, 2025
Merged

Performance fix#529
rubenvdlinde merged 1 commit into
developmentfrom
hotfix/settingspage

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

No description provided.

@rubenvdlinde
rubenvdlinde merged commit 809e52a into development Aug 13, 2025
1 of 2 checks passed
remko48 pushed a commit that referenced this pull request Aug 13, 2025
Performance fix

  Restored beta version to 0.2.6-beta.4
rubenvdlinde added a commit that referenced this pull request Dec 28, 2025
rubenvdlinde added a commit that referenced this pull request Aug 12, 2026
…ating it (#2439)

* fix(tests): pull in the helper trait four Flow tests use but never require

`composer test:unit` FATALS outside a Nextcloud checkout:

    Fatal error: Trait "…\FiltersFlowLevelFindings" not found
    in tests/Unit/Service/Flow/FlowNodeConfigDialectTest.php on line 61

Not a new break — it reproduces on an untouched `development`. The suite
matches `*Test.php`, so a helper that is not itself a test case is never
included, and composer declares no `autoload-dev`, so nothing autoloads the
test namespace either. In CI the trait resolves only because `tests/bootstrap.php`
requires the SERVER's `tests/autoload.php` when it can find a Nextcloud root.

The cost was not the four files: PHPUnit aborts the whole run at the first
fatal, so the ENTIRE 16,303-test unit suite was unrunnable locally, and the
51 tests in these four files never ran anywhere the server autoloader was
absent.

Fixed the way the AppHost and Mcp fixtures in this repo already do it — an
explicit `require_once` in each consumer. An `autoload-dev` PSR-4 rule was
tried first and rejected twice over: 572 test classes use namespaces that do
not correspond to their paths (composer prints a skip warning for each), and
a `classmap` over `tests/` would make `tests/stubs/`'s OCP stubs autoloadable,
which has bricked an instance here before.

Verified: 16,303 tests, 0 failures, 0 errors.

* style: clear the two pre-existing phpcs errors on development

`lib/AppInfo/Application.php` indented a quoted error message three spaces
inside a `//` comment run; `ObjectWriteNode.php` had a 166-character
translatable string on one line.

The long line is wrapped as concatenated literals inside `t()` — the form
already used elsewhere in the same file (line ~1264), so the extractor still
sees a literal. Assigning the message to a variable first would have hidden
it from translation extraction entirely.

* feat(flow): give a flow's rationale a column, so the database stops eating it

A flow authored as a definition file carries its reasoning in a top-level
`$comment`. On hydra's lock reaper that is 90 lines recording four defects and
what prevents each recurring. `openregister_flows` had no column for it, so
importing such a file and regenerating it FROM the database returned a flow
without that text — silently, because a flow with no rationale looks exactly
like one whose author wrote none.

The standing workaround was to regenerate by MERGING file and database rather
than exporting. That kept the text alive but made the FILE its only home: a
flow edited through the UI could not carry a rationale at all, and two authors
working on the same flow by different routes disagreed about why it was shaped
that way with nothing to reconcile them.

- `comment` TEXT on `openregister_flows`, nullable. TEXT because the existing
  bodies already exceed 6,000 characters and a length-capped column would
  truncate on write rather than refuse it.
- `Flow::$comment`, in `addType()` and in `jsonSerialize()`.
- `FlowService::applyEditableFields()` accepts `comment`, and normalises the
  `$comment` a definition file actually uses — that key cannot be a column
  name, so it is translated once here rather than at every call site. An
  explicit `comment` wins over the alias: a UI edit is a deliberate write,
  `$comment` is whatever the file happened to carry.

Distinct from the two fields it sits next to: `description` is the one-line
label the UI lists a flow by, `notes` is a working scratchpad.

Six tests, positive-controlled: removing the field mapping turns four of them
red, and the fifth (a partial update must not blank a stored comment) is
deliberately driven through the UPDATE path — on a create the field starts
null, so the same assertion there would hold no matter what the code did. Its
counterpart pins that an explicit null still clears the field.

Closes the decision recorded on #529.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants