[Telemetry Table] Address issues found during testing Table Performance #7529

jvigliotta · 2024-02-28T00:36:41Z

Describe your changes:

Fix exporting from Limited Mode: #7268 (comment)
Fix UI issues: #7268 (comment)
Apply configuration changes made in Edit Properties.

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this a notable change that will require a special callout in the release notes Notable Change ? For example, will this break compatibility with existing APIs or projects which source these plugins?

Author Checklist

Changes address original issue?
Tests included and/or updated with changes? (issue created: Recent Table Performance Changes Need e2e tests #7419 )
Has this been smoke tested?
Have you associated this PR with a type: label? Note: this is not necessarily the same as the original issue.
Have you associated a milestone with this PR? Note: leave blank if unsure.
Is this a breaking change to be called out in the release notes?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?

codecov · 2024-02-28T00:41:56Z

Codecov Report

Attention: Patch coverage is 42.85714% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 56.02%. Comparing base (cb4c59a) to head (2de4c21).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7529      +/-   ##
==========================================
- Coverage   56.36%   56.02%   -0.34%     
==========================================
  Files         672      672              
  Lines       27094    27108      +14     
  Branches     2624     2629       +5     
==========================================
- Hits        15272    15188      -84     
- Misses      11500    11598      +98     
  Partials      322      322

Flag	Coverage Δ
e2e-full	`23.60% <33.33%> (-18.10%)`	⬇️
e2e-stable	`59.66% <66.66%> (-0.10%)`	⬇️
unit	`49.13% <42.85%> (+<0.01%)`	⬆️

Files	Coverage Δ
src/plugins/telemetryTable/TelemetryTable.js	`80.78% <100.00%> (+0.19%)`	⬆️
src/plugins/telemetryTable/TelemetryTableType.js	`100.00% <100.00%> (ø)`
src/plugins/telemetryTable/components/TableRow.vue	`35.71% <100.00%> (+1.56%)`	⬆️
...telemetryTable/components/TableFooterIndicator.vue	`28.57% <0.00%> (ø)`
...ugins/telemetryTable/components/TableComponent.vue	`41.84% <38.09%> (-0.66%)`	⬇️

... and 10 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb4c59a...2de4c21. Read the comment docs.

ozyx

Needs e2e 🧪 or identify the existing e2e 🧪 that covers this

akhenry · 2024-03-06T18:37:38Z

e2e/tests/functional/plugins/telemetryTable/telemetryTable.e2e.spec.js

+    await page.goto(table.url);
+    await setTimeConductorMode(page, false);
+    const rows = page.getByRole('row');
+    await expect(rows).toHaveCount(50);


akhenry · 2024-03-11T21:26:43Z

Actually showing 51 rows, but says 50
Table throws error in real-time

jvigliotta · 2024-03-12T20:43:00Z

src/plugins/telemetryTable/components/TableComponent.vue

+
+      this.persistModeChange = persistModeChange;
+
+      if (this.rowLimit !== rowLimit) {


@akhenry this was previously this.rowLimit === rowLimit which was causing duplicate unsubscribes with the second unsubscribe failing because it was already removed in the realtime-provider.

jvigliotta · 2024-03-12T20:43:28Z

src/plugins/telemetryTable/components/TableComponent.vue

@@ -1038,7 +1064,7 @@ export default {
          let row = allRows[i];
          row.marked = true;

-          if (row !== baseRow) {
+          if (row !== baseRow && this.markedRows.indexOf(row) === -1) {


we were getting dupes in markedRows which caused the count to be off

unlikelyzero · 2024-03-12T21:39:20Z

e2e/tests/functional/plugins/telemetryTable/telemetryTable.e2e.spec.js

+      parent: table.uuid
+    });
+    await page.goto(table.url);
+    await setTimeConductorMode(page, false);


this is fine, but we should we try to avoid this appaction

…e callbacks after loading is finished

jvigliotta added the type:bug label Feb 28, 2024

jvigliotta added this to the Target:4.0.0 milestone Feb 28, 2024

ozyx self-requested a review February 28, 2024 01:01

ozyx suggested changes Mar 4, 2024

View reviewed changes

akhenry self-requested a review March 4, 2024 21:51

akhenry approved these changes Mar 8, 2024

View reviewed changes

jvigliotta requested a review from ozyx March 12, 2024 20:41

jvigliotta commented Mar 12, 2024

View reviewed changes

ozyx approved these changes Mar 12, 2024

View reviewed changes

unlikelyzero reviewed Mar 12, 2024

View reviewed changes

jvigliotta and others added 15 commits March 12, 2024 15:02

mostly cosmetic and configuration setup changes

04916f0

temp sourcemaps

0509933

add a new after load actions array for more precise ability to execut…

fe28069

…e callbacks after loading is finished

applying changes made in Edit Properties

bb766ab

revert source maps

fc36b02

test(e2e): add minimal e2e test

337b243

source maps

a9533a5

wrong logic

aa2370a

debuggin row count

28c7eff

checking for dupes in marked rows

6d5ad13

removed debug source maps

8ee9409

removing row counter debug code

7e049de

test: fix locator

1507fa5

test: fix locator

18a6b47

css: fix pointer event interception

5791c20

ozyx force-pushed the table-performance-fixes branch from 5d50b2c to 5791c20 Compare March 12, 2024 23:08

fix: test

1c7ac17

akhenry added the pr:e2e:couchdb npm run test:e2e:couchdb label Mar 13, 2024

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Mar 13, 2024

Merge branch 'master' into table-performance-fixes

2de4c21

akhenry added the pr:e2e:couchdb npm run test:e2e:couchdb label Mar 13, 2024

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Mar 13, 2024

akhenry merged commit 6486263 into master Mar 13, 2024
36 checks passed

akhenry deleted the table-performance-fixes branch March 13, 2024 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Telemetry Table] Address issues found during testing Table Performance #7529

[Telemetry Table] Address issues found during testing Table Performance #7529

jvigliotta commented Feb 28, 2024 •

edited by akhenry

Loading

codecov bot commented Feb 28, 2024 •

edited

Loading

ozyx left a comment •

edited

Loading

akhenry Mar 6, 2024

akhenry commented Mar 11, 2024

jvigliotta Mar 12, 2024

jvigliotta Mar 12, 2024

unlikelyzero Mar 12, 2024


		this.persistModeChange = persistModeChange;

		if (this.rowLimit !== rowLimit) {

[Telemetry Table] Address issues found during testing Table Performance #7529

[Telemetry Table] Address issues found during testing Table Performance #7529

Conversation

jvigliotta commented Feb 28, 2024 • edited by akhenry Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

codecov bot commented Feb 28, 2024 • edited Loading

Codecov Report

ozyx left a comment • edited Loading

Choose a reason for hiding this comment

akhenry Mar 6, 2024

Choose a reason for hiding this comment

akhenry commented Mar 11, 2024

jvigliotta Mar 12, 2024

Choose a reason for hiding this comment

jvigliotta Mar 12, 2024

Choose a reason for hiding this comment

unlikelyzero Mar 12, 2024

Choose a reason for hiding this comment

jvigliotta commented Feb 28, 2024 •

edited by akhenry

Loading

codecov bot commented Feb 28, 2024 •

edited

Loading

ozyx left a comment •

edited

Loading