Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(archives): fix duplicate archived recording displays #820

Merged
merged 2 commits into from
Jan 17, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Jan 17, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Related to #811

Description of the change:

  • Fix hook deps for archive table target props.
  • Fix hook deps for All Targets table.
  • Fix count number updates for All Targets table (missing listener for ArchivedRecordingCreated type).

Motivation for the change:

After some previous refactoring jobs, the Archive table now accepts a target prop that select a target to pull archive from. However, that seems to introduce problems with hook deps.

#811 fixes the hook deps issue for upload table but Archive table & All Targets (additionally, count number is not updated correctly) suffers the same problem. All Archives table seems to be safe since it refreshes everything on any updates.

This leads to duplicate archive recording rows when testing with https://github.com/cryostatio/cryostat/pull/1333

Signed-off-by: Thuan Vo <thvo@redhat.com>
@tthvo tthvo added the fix label Jan 17, 2023
@mergify mergify bot added the safe-to-test label Jan 17, 2023
@tthvo tthvo marked this pull request as draft January 17, 2023 01:42
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-820-7971b9fa5ec6e0ab37af6b9bbc5ca5e40691cb8c sh smoketest.sh

@tthvo tthvo marked this pull request as ready for review January 17, 2023 03:35
@tthvo
Copy link
Member Author

tthvo commented Jan 17, 2023

There is a bit of diffculty with MODIFIED event as target's connectUrl is reserved but something else might change (i.e. jvmId). This still makes the observable prop changes.

So as a safeguard, I add a filter in ArchiveRecordingTable event listener so that it remove duplicates if any, instead of just concat new recording into rows.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-820-5e533de823d00d7ebe23c1fb372e2461a130bad2 sh smoketest.sh

@andrewazores andrewazores merged commit 3193f1c into cryostatio:main Jan 17, 2023
@tthvo tthvo deleted the fix-archive-table-update branch January 17, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants