refactor(Jobs): replace old table with dynamic mat table

[abdimo101]

Description

This PR replaces the old jobs table with dynamic mat table & jobs columns to externalSettings

Before:

After:

Motivation

Background on use case, changes needed

Fixes:

Please provide a list of the fixes implemented in this PR

• Items added

Changes:

Please provide a list of the changes implemented by this PR

• changes made

Tests included

• Included for each change/fix?
• Passing? (Merge will not be approved unless this is checked)

Documentation

• swagger documentation updated [required]
• official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

• Does it require a specific version of the backend
• which version of the backend is required:

Summary by Sourcery

Refactor the jobs dashboard to use the shared dynamic material table with user-configurable column settings and server-side pagination, integrating persisted user settings for job table columns.

New Features:

• Introduce a dynamic jobs table backed by a configurable table settings model, including pagination, row events, and global text search for jobs.
• Persist user-specific job table column configurations via the centralized user settings store and selectors, enabling personalized job table layouts.

Enhancements:

• Wire the jobs dashboard to the NgRx store with a dedicated page view model selector that combines jobs data, filters, counts, and table settings.
• Align settings models and configuration metadata to support the new job table column settings scope.
• Improve jobs dashboard tests to cover navigation from row click events and to use store-based test setup.

Tests:

• Extend unit tests for the jobs dashboard component to verify row click navigation and the new store-backed setup.
• Update user settings effects tests to account for the new job table column settings field.

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• The component now keeps both a BehaviorSubject<OutputJobV3Dto[]> and a SciCatDataSource; consider unifying on a single, strongly-typed data source interface to avoid duplicated state and reduce the risk of pagination/count getting out of sync.
• currentFilters and the filters argument in loadData are typed as any; defining a dedicated filters interface (with skip, limit, sortField, globalSearch, etc.) will make the data loading and pagination logic easier to reason about and safer to change.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The component now keeps both a `BehaviorSubject<OutputJobV3Dto[]>` and a `SciCatDataSource`; consider unifying on a single, strongly-typed data source interface to avoid duplicated state and reduce the risk of pagination/count getting out of sync.
- `currentFilters` and the `filters` argument in `loadData` are typed as `any`; defining a dedicated filters interface (with `skip`, `limit`, `sortField`, `globalSearch`, etc.) will make the data loading and pagination logic easier to reason about and safer to change.

## Individual Comments

### Comment 1
<location path="src/app/jobs/jobs-dashboard-new/jobs-dashboard-new.component.ts" line_range="124-133" />
<code_context>
+    this.currentFilters = newFilters;
+  }
+
+  ngOnDestroy() {
+    this.subscriptions.forEach((sub) => sub.unsubscribe());
   }
 }
</code_context>
<issue_to_address>
**issue (bug_risk):** SciCatDataSource cleanup is no longer called on destroy

`SciCatDataSource` is still used (`this.scicatDataSource`), but its explicit teardown (e.g. `disconnectExportData()` or similar) is no longer invoked in `ngOnDestroy`. If the data source manages resources or internal subscriptions, skipping this cleanup can cause leaks. Please restore the appropriate disconnect/cleanup call in `ngOnDestroy` in addition to unsubscribing from `subscriptions`.
</issue_to_address>

### Comment 2
<location path="src/app/jobs/jobs-dashboard-new/jobs-dashboard-new.component.ts" line_range="188-198" />
<code_context>
+    }
+  }
+
+  onPaginationChange(pagination: TablePagination) {
+    const pageIndex = pagination.pageIndex;
+    const pageSize = pagination.pageSize;
+    const newFilters = {
+      ...this.currentFilters,
+      skip: pageIndex * pageSize,
+      limit: pageSize,
+    };
+
+    this.loadData(newFilters, pageIndex, pageSize);
+  }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** currentFilters is not updated on pagination changes, which can desync with the active filters

In `onPaginationChange`, `newFilters` is derived from `this.currentFilters` and passed to `loadData`, but `this.currentFilters` itself is never updated. Unlike `onGlobalTextSearchAction`, this leaves `currentFilters` out of sync with the last pagination state, which can break any logic that treats it as the source of truth. Please also assign `this.currentFilters = newFilters;` here.

```suggestion
  onPaginationChange(pagination: TablePagination) {
    const pageIndex = pagination.pageIndex;
    const pageSize = pagination.pageSize;
    const newFilters = {
      ...this.currentFilters,
      skip: pageIndex * pageSize,
      limit: pageSize,
    };

    this.currentFilters = newFilters;
    this.loadData(newFilters, pageIndex, pageSize);
  }
```
</issue_to_address>

### Comment 3
<location path="src/app/jobs/jobs-dashboard-new/jobs-dashboard-new.component.spec.ts" line_range="13" />
<code_context>
+import { NO_ERRORS_SCHEMA } from "@angular/core";
+import { StoreModule, Store } from "@ngrx/store";
+import { Router } from "@angular/router";
+import { FlexLayoutModule } from "@ngbracket/ngx-layout";
+import { MatButtonModule } from "@angular/material/button";
+import { MatIconModule } from "@angular/material/icon";
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the test setup by removing unnecessary modules, indirections, and unused mocks while keeping only the minimal configuration needed for the new behavior tests.

You can keep the new behavior tests while significantly simplifying the spec setup.

### 1. Drop unused modules and real Store

`FlexLayoutModule`, `MatButtonModule`, `MatIconModule`, and `StoreModule.forRoot({})` are not needed for:

- component creation
- verifying `onRowEvent` navigates correctly

You can remove them from `imports` entirely:

```ts
beforeEach(waitForAsync(() => {
  TestBed.configureTestingModule({
    schemas: [NO_ERRORS_SCHEMA],
    declarations: [JobsDashboardNewComponent],
    providers: [
      { provide: Router, useValue: router },
      { provide: AppConfigService, useValue: { getConfig } },
      { provide: ScicatDataService, useValue: {} },
      { provide: ExportExcelService, useValue: {} },
      provideMockStore({
        selectors: [
          { selector: selectJobsDashboardPageViewModel, value: jobsVm },
        ],
      }),
    ],
  }).compileComponents();
}));
```

This keeps the mock store and all functionality but removes the unused UI/store modules.

### 2. Remove `overrideComponent` indirection

You don’t need `TestBed.overrideComponent` here because you’re not overriding providers declared in the component metadata; you’re just supplying dependencies. Moving providers into `configureTestingModule` (as above) makes the test more direct and easier to follow.

So you can delete:

```ts
TestBed.overrideComponent(JobsDashboardNewComponent, {
  set: {
    providers: [ /* ... */ ],
  },
});

TestBed.compileComponents();
```

and rely on `compileComponents()` from `configureTestingModule` instead.

### 3. Remove unused `store` injection

The `beforeEach(inject([Store], ...))` block assigns `store` but never uses it. You can safely remove it without changing test behavior:

```ts
// Remove this block entirely:
beforeEach(inject([Store], (mockStore: MockStore) => {
  store = mockStore;
}));
```

If you later need to interact with the mock store, you can always reintroduce it.

### 4. Minimize the selector mock to what’s actually required

If the component only requires `vm$` to exist for initialization, you can slim down `jobsVm` to the minimum shape used in `ngOnInit`/template, rather than a full domain object:

```ts
const jobsVm = {
  jobs: [],
  count: 0,
  // add only the fields the component actually reads
};
```

This keeps the ngrx wiring intact but makes the test’s intent clearer and the setup lighter. If you add tests that exercise pagination/columns later, you can expand `jobsVm` as needed.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}