Skip to content

refactor(Sample-Datasets): replace old table with dynamic-mat-table and fix pagination bugs#2353

Open
abdimo101 wants to merge 4 commits intomasterfrom
refactor-sample-datasets-dynamic-table
Open

refactor(Sample-Datasets): replace old table with dynamic-mat-table and fix pagination bugs#2353
abdimo101 wants to merge 4 commits intomasterfrom
refactor-sample-datasets-dynamic-table

Conversation

@abdimo101
Copy link
Copy Markdown
Member

@abdimo101 abdimo101 commented Apr 27, 2026

Description

This PR replaces the old sample datasets table with dynamic-mat-table and fixes pagination bugs.

Before:
image

After:
image

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

Replace the sample detail datasets table with the shared dynamic material table and align dataset fetching and pagination with server-side APIs.

New Features:

  • Introduce a dynamic material table for displaying related datasets in the sample detail view, including empty-state messaging and configurable pagination.

Bug Fixes:

  • Fix incorrect datasets count retrieval by switching to the dedicated count endpoint.
  • Resolve pagination inconsistencies by wiring dynamic table pagination to the NgRx store and backend server-side pagination parameters.
  • Fix row navigation by handling dynamic table row click events and using dataset PID from the table row data.

Enhancements:

  • Standardize table column definitions and styling using shared dynamic-material-table models.
  • Refine related datasets tab labelling and row styling for improved clarity and consistency.

@abdimo101 abdimo101 requested a review from a team as a code owner April 27, 2026 14:34
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • You now have multiple subscriptions to vm$ in SampleDetailComponent that each handle different concerns (attachments, table data, pagination); consider consolidating these into a single subscription or using tap in the selector to reduce duplicate subscription logic and make state updates easier to reason about.
  • Instead of maintaining a BehaviorSubject<TableData[]> and manually calling next on each vm$ update, you could expose an observable data source and bind to it directly in the template (e.g., via async pipe), which would simplify the component and reduce state duplication.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- You now have multiple subscriptions to `vm$` in `SampleDetailComponent` that each handle different concerns (attachments, table data, pagination); consider consolidating these into a single subscription or using `tap` in the selector to reduce duplicate subscription logic and make state updates easier to reason about.
- Instead of maintaining a `BehaviorSubject<TableData[]>` and manually calling `next` on each `vm$` update, you could expose an observable data source and bind to it directly in the template (e.g., via `async` pipe), which would simplify the component and reduce state duplication.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@abdimo101 abdimo101 requested review from Junjiequan and nitrosx April 29, 2026 09:06
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.

1 participant