The performance decreased because of a combination of calls that were supposed to increase performance.
We have a mechanism in place which automatically eager loads models needed in the element representers when a collection of them is rendered. This is to avoid N+1 queries of course. But, if eager loading is combined with e.g., a LIMIT, which we do because we paginate, rails automatically falls back to issuing two instead of just one SQL statement. Which makes sense as otherwise LEFT JOINS might mess with the result set.
But Rails does so in a somewhat simple fashion. It uses the first query to get the DISTINCT ids. The second is used to load the values (without a limit). But instead of removing all WHERE statements in the second SQL statement and then apply just the one for the ids, it keeps the original WHERE statement and applies the one for the ids on top. The problem with that is that the database trips on that (I didn't check the why) and uses a less than optimal query plan.
That was the problem here as well. The first query remained reasonable quick (300ms) but the second one took 25s.
The fix is to split the two statements by hand in the representer whenever eager loading is defined. The first query has all the filters but no eager loading and fetches the ids. The second takes the ids, and with eager loading included loads the data. Et voila, second query takes 10ms.
That at least works for relations, work_packages and projects. But there are other representers that also seem to have custom behaviour. I'm looking into whether they can be easily adapted.
The ancestors/children representer change calls `child.display_id`,
which consults `identifier` in semantic mode. The Hierarchy eager
loader preloads children with a minimal `SELECT id, subject, project_id,
parent_id` for performance, so `identifier` was missing and
`ActiveModel::MissingAttributeError` fired the moment a query endpoint
rendered a work package with visible children in semantic mode.
Add `identifier` to the SELECT. It's one extra short text column per
child row.
In semantic mode, the work package breadcrumb renders numeric IDs
instead of the semantic identifier because ancestor HAL resources are
built from `_links.ancestors[]` entries that only carry `href` and
`title`. With no top-level `displayId` in `$source`, the frontend getter
falls through to the numeric id parsed from the href.
Emit `displayId` alongside `href`/`title` on each ancestor and child
link in the representer, and have the `displayId` getter fall back to
the self link's `displayId` so resources built from a link payload
alone still surface the semantic identifier.
Restore the minimal admin settings blankslate so the admin menu route
remains valid after the sprint-based cleanup. Remove the remaining
settings-driven story/task classification code, dead models and
services, and the obsolete filter and spec setup that depended on it.
Overriding wp.id to return the semantic identifier (e.g. "PROJ-42")
broke cache keys, API filters, row rendering, and CSS selectors that
all depend on the numeric PK.
Instead, keep wp.id as the numeric PK and add two new properties:
- displayId: returns the user-facing identifier ("PROJ-42" or "123")
- displayIdWithHash: returns "#PROJ-42" or "#123" for UI display
Also adds a COALESCE fallback in the SQL representer so work packages
created before semantic mode was enabled still get a valid displayId.
Extract FinderMethods module that transparently resolves both numeric and
semantic identifiers (e.g. "PROJ-42") using FriendlyId's Object#friendly_id?
for dispatch. The module is included in both the WorkPackage class and
extended onto every relation, so scoped queries like
WorkPackage.visible(user).find("PROJ-42") work seamlessly.
- Override find to resolve semantic IDs via identifier column + alias table
- Override exists? with the same resolution chain
- Refactor find_by_id_or_identifier to use friendly_id? instead of semantic_id?
- Update API route to accept string IDs (type: Integer → type: String)
- Update controller and ViewComponent finders to use find_by_id_or_identifier
- Pass display_id from Rails views to Angular custom elements
Replace the conditional `semanticId` API field with `displayId` which is
always present in work package responses. In semantic mode it returns the
project-based identifier (e.g. "PROJ-42"), in classic mode it returns the
numeric ID as a string. This gives API consumers (frontend, mobile) a
single field to read without conditional logic.
- Add `WorkPackage#display_id` method that encapsulates the mode check
- Update both representers (JSON and SQL) to render `displayId` unconditionally
- Update OpenAPI schema documentation
Adds the computed semanticId property to the HAL representer,
SQL collection representer, and schema representer. The property is
gated behind the semantic_work_package_ids feature flag and returns the
value from WorkPackage#identifier. Includes OpenAPI docs
and the translation key for the schema name.
Add fetch_request_cached method that layers RequestStore in front of Rails.cache.fetch
Used in the following places, as they are repeatedly accessed during schema initialization.
all_work_package_form_attributes, form_config_attribute_representation, Query.available_columns
In my tests, this improves cold cache access by reducing initial number of queries to access cache
This error is intended for cases when a method is
intentionally not implemented, because the module/class defining
it expects a subclass (or class including the module) to implement
the method.
This is intended to distinguish it from other cases, such as:
* feature not implemented yet
* edge case of a method call not yet supported
Notably it avoids the misuse of the Ruby-defined NotImplementedError,
which is only intended for much more specific scenarios:
> Raised when a feature is not implemented on the current platform. For example, methods depending on the fsync or fork system calls may raise this exception [...]
Also see https://docs.ruby-lang.org/en/master/NotImplementedError.html
This avoids having to reset the subject first and having to save the automatic subject in an after_perform.
This is especially useful if no change is performed at all. Without the change, even without a change saving would lead to a new journal
This is against the schema for definition for links, which
already allow the title to be missing, but don't allow
it to be null. If it's present it must have a string as a value.
Those self-tests are "basic" in the sense that they only validate
their compliance with our documented schema in one representation.
These test cases don't yet cover/validate whether the generated
representation also fulfills the schema under different circumstances,
for example when rendering for a user with fewer privileges, not allowed
to see certain fields.
Where necessary, the schema was changed to reflect the reality, e.g.
when those tests revealed that a "required" field might be missing due to
a lack of permissions.
In a few cases the implementation was adapted to allow for stricter guarantees
of the specified schema, for example links allowed to leave out the title key
already, so its not necessary to emit `title: nil` in cases where a title is
not known.
Index endpoints will automatically discover and use
the `to_eager_load` and `to_preload` declarations of their
representers. This takes away the possibility of forgetting
to add those to the scope.
I found a few endpoint that didn't use them, when other endpoints
rendering the same collection did.
Interestingly the SCM integration modules consistently declared
those methods but never used them.
This allows to make more efficient database queries in the API
and reduces overhead by too large JSON responses.
Assuming we fix#68457, additional improvements in response time
can be expected, because we do not need to perform a count for all
matching work packages anymore (because we do not select "total").
Tomas Hykel