[O2B-1534] Migrate log overview to use filtering model pattern by NarrowsProjects · Pull Request #2083 · AliceO2Group/Bookkeeping

NarrowsProjects · 2026-02-20T11:53:37Z

I have a JIRA ticket

branch and/or PR name(s) include(s) JIRA ID
issue has "Fix version" assigned
issue "Status" is set to "In review"
PR labels are selected

Notable changes for users:

Individual from-to selectors on the logs page have been replaced with a single field that will open the standard time range selector
All open-input forms apply their filters when focus is lost on them.

Notable changes for developers:

All filters have been moved to a filteringModel in logsOverviewModel
The filters for runs, lhcfills and environments have been renamed under the hood to match getAllRunsUseCase:
- runs => runNumbers
- lhcFills => fillNumbers
- environments => environmentIds

Changes made to the database:

none

codecov · 2026-02-20T11:55:29Z

Codecov Report

❌ Patch coverage is 34.78261% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.75%. Comparing base (1333378) to head (3bf7e66).

Files with missing lines	Patch %	Lines
...ib/public/views/Logs/Overview/LogsOverviewModel.js	0.00%	11 Missing ⚠️
...blic/views/Logs/ActiveColumns/logsActiveColumns.js	0.00%	8 Missing ⚠️
...nts/Filters/LogsFilter/author/AuthorFilterModel.js	0.00%	6 Missing ⚠️
...mponents/Filters/LogsFilter/author/authorFilter.js	0.00%	3 Missing ⚠️
...ic/components/Filters/common/filters/textFilter.js	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2083      +/-   ##
==========================================
+ Coverage   45.50%   45.75%   +0.24%     
==========================================
  Files        1044     1042       -2     
  Lines       17353    17213     -140     
  Branches     3149     3119      -30     
==========================================
- Hits         7897     7875      -22     
+ Misses       9456     9338     -118

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… anything

…FilteringModel-pattern

isaachilly · 2026-04-13T09:35:00Z

lib/public/components/Filters/LogsFilter/author/authorFilter.js

 *
- * @param {LogModel} logModel the log model object
- * @returns {Component} the author filter component
+ * @param {LogsOverviewModel} logsOverviewModel the log overview model


The overviewModel param doesn't exist in the func.

Taken care of

isaachilly · 2026-04-13T09:42:57Z

lib/public/components/Filters/LogsFilter/author/authorFilter.js

-    authorFilterTextInput(authorFilter),
-    resetAuthorFilterButton(authorFilter),
-    excludeAnonymousLogAuthorToggle(authorFilter),
+export const authorFilter = ({ filteringModel }) => h('.flex-row.items-center.g3', [


It would be more consistent with other ActiveColumns Filter setups to get the matching filter's model in ActiveColumns and pass only that to the Filter. And we sent the function only the specific data it needs.

Taken care of

isaachilly · 2026-04-13T10:01:00Z

lib/public/components/Filters/LogsFilter/author/authorFilter.js

 const resetAuthorFilterButton = (authorFilterModel) => h(
    '.btn.btn-pill.f7',
-    { disabled: authorFilterModel.isEmpty, onclick: () => authorFilterModel.clear() },
+    { disabled: authorFilterModel.isEmpty, onclick: () => authorFilterModel.reset() },


.clear() reloaded the table's items because it notified. This is not the case currently with .reset() of RawTextFilterModel thus, when you click the cross button, the author text is emptied, but the table remains the same, it is not refreshed as it should be.

Taken care off

…nes if the reset function also notifies the observers

isaachilly · 2026-04-13T10:49:28Z

lib/public/components/Filters/common/filters/RawTextFilterModel.js

     * @inheritDoc
     */
-    reset() {
+    reset(notify = false) {


TBD offline with result commented here.

…and call notify there

isaachilly · 2026-04-13T10:55:44Z

lib/public/components/Filters/LogsFilter/author/authorFilter.js

-    iconX(),
-);
+const resetAuthorFilterButton = (authorFilterModel) =>
+    h('.btn.btn-pill.f7', { disabled: authorFilterModel.isEmpty, onclick: () => authorFilterModel.reset(true) }, iconX());


True is still being sent to the function here, we are now overwriting it so don't need the param.

Thanks for the catch

isaachilly · 2026-04-13T11:05:28Z

lib/public/components/Filters/LogsFilter/author/AuthorFilterModel.js

-            return;
-        }
-
+    reset() {


I think the clear method just needs reinstating because it handled the if empty and the notify. This will create a duplicate fetch because filteringModel will call reset on the model and it'll notify.

Taken care of

isaachilly · 2026-04-13T12:07:42Z

lib/public/views/Logs/ActiveColumns/logsActiveColumns.js

-        ),
+
+        /**
+         * Title filter component


Title, contend and author do not have placeholder text and I think it would be good and consistent for them to have.

Taken care of

isaachilly · 2026-04-13T13:26:14Z

lib/usecases/log/GetAllLogsUseCase.js

    }

-    if (filter.run?.values?.length > 0) {
+    if (runNumbers?.length > 0) {


Can this just be if runNumbers? Check the other filter ifs.

I suppose you are right, I just didn't question the original logic

isaachilly · 2026-04-13T13:51:28Z

test/public/logs/overview.test.js


-        await pressElement(page, '#reset-filters');
+        await waitForTableLength(page, 1);
+        await openFilteringPanel(page);


Is this line needed?

Yes, but the line shouldn't be there, the next text needs the filteringpanel to be open

isaachilly · 2026-04-13T13:52:03Z

test/public/logs/overview.test.js

-        await pressElement(page, '#reset-filters');
+        await waitForTableLength(page, 1);
+        await openFilteringPanel(page);
+        await resetFilters(page);


Could we make use of before/after each functions to handle resetting filters etc?

Taken care of

isaachilly · 2026-04-13T15:05:11Z

test/public/logs/overview.test.js

        await pressElement(page, '#openFilterToggle');
        await waitForTableTotalRowsCountToEqual(page, 119);
-
+        // await new Promise((res, _rej) => setTimeout(() => res(), 2000))


I'm assuming leftover things here.

isaachilly · 2026-04-13T15:06:07Z

test/public/logs/overview.test.js

-        await waitForNavigation(page, () => pressElement(page, `${popoverSelector} a`))
-        expectUrlParams(page, { page: 'run-detail', runNumber: 2 })
-    });
+    // it('can filter by creation date', async () => {


An accidental commit of commented out tests.

…for length

Guust added 4 commits February 20, 2026 10:23

chore: remove redundant model assignment.

722caf7

chore: move all filters denoted as so to a filteringModel

f161cae

Make filterInputModel extend filterModel

9faffaf

rename inputfilter to ParsedInputFilter

247943b

NarrowsProjects self-assigned this Feb 20, 2026

NarrowsProjects requested a review from graduta as a code owner February 20, 2026 11:53

NarrowsProjects added frontend javascript Pull requests that update Javascript code labels Feb 20, 2026

NarrowsProjects marked this pull request as draft February 20, 2026 11:53

implement authorfilter with ParsedInputFilterModel changes

114285d

NarrowsProjects force-pushed the improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern branch from 07567e2 to 114285d Compare February 20, 2026 12:02

Guust added 9 commits February 20, 2026 13:18

re-implement filter

f39f7ae

add tag-filters to the filtering object

df1fea3

replace titleFilter and contentFilter with RawTextFilterModels

96ca85f

make Authorfilter an implementation of RawTextFilterModel

93b3291

add runs filter to the filteringmodel

5a5e830

add environments filter to the filteringmodel

b5e7148

add lhcFills filter to the filteringmodel

3507ba0

add created filter to the filteringmodel

9c63409

move the sort 'filter' to fetchlogs, since it doesn't actually filter…

5500b3e

… anything

NarrowsProjects force-pushed the improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern branch from a936a03 to 5500b3e Compare February 20, 2026 16:24

Guust added 7 commits February 21, 2026 16:57

change filter to rawTextFilter for title

a09ae8b

fix test by changing event type to 'change'

512999b

fix content and author tests

0f15812

import openFilteringPanel and resetFilters

7b641ca

fix createdAt filter test

5ab8218

change event types to change

4c96d73

fix isAnyFilterActive

18ce4dc

NarrowsProjects marked this pull request as ready for review February 21, 2026 18:22

NarrowsProjects added 4 commits March 9, 2026 11:06

Merge branch 'main' into improv/O2B-1534/Migrate-Log-Overview-to-use-…

237683a

…FilteringModel-pattern

Merge branch 'main' into improv/O2B-1534/Migrate-Log-Overview-to-use-…

a222599

…FilteringModel-pattern

Merge branch 'main' into improv/O2B-1534/Migrate-Log-Overview-to-use-…

7a5601f

…FilteringModel-pattern

Merge branch 'main' into improv/O2B-1534/Migrate-Log-Overview-to-use-…

12dba35

…FilteringModel-pattern