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

feat(#7367): ProgressBar for ExportAsJSONAction #7374

Merged
merged 20 commits into from
Jan 18, 2024

Conversation

ozyx
Copy link
Contributor

@ozyx ozyx commented Jan 12, 2024

Closes #7367
Closes #7380

Describe your changes:

Adds a ProgressBar to asynchronous Export as JSON actions (such as with external persistence i.e. CouchDB).

Also a few refactors and minor fixes:

  • Updates the URLs for example imagery
  • Updates LocalStorageProvider to use the actual localStorage api (getItem() and setItem(), etc).
  • Updates ExportAsJSONAction to use private methods
  • Refactors out the model prop object in ProgressDialog-- use individual props instead
Screen.Recording.2024-01-12.at.11.58.02.AM.mov

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 change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Tests included and/or updated with changes?
  • 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?

@ozyx ozyx added this to the Target:3.3.0 milestone Jan 12, 2024
@@ -71,14 +71,14 @@ export default class LocalStorageObjectProvider {
* @private
*/
persistSpace(space) {
this.localStorage[this.spaceKey] = JSON.stringify(space);
this.localStorage.setItem(this.spaceKey, JSON.stringify(space));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a driveby, but figured we should be using the localStorage API here

Copy link

deploysentinel bot commented Jan 12, 2024

Current Playwright Test Results Summary

✅ 174 Passing - ❌ 1 Failing - ⚠️ 2 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 01/12/2024 09:34:41pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 655cb39

Started: 01/12/2024 09:29:00pm UTC

❌ Failures

📄   functional/plugins/plot/logPlot.e2e.spec.js • 1 Failure

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Log plot tests Log Plot ticks are functionally correct in regular and log mode and after refresh
Retry 2Retry 1Initial Attempt
Error: locator.click: Error: strict mode violation: locator('div:nth-child(5) .c-form-r...
locator.click: Error: strict mode violation: locator('div:nth-child(5) .c-form-row__controls .form-control .field input') resolved to 11 elements:
    1) <input name="name" type="text" id="form-name"/> aka getByLabel('Title', { exact: true })
    2) <input type="number" aria-label="Period"/> aka getByLabel('Period')
    3) <input type="number" aria-label="Amplitude"/> aka getByLabel('Amplitude')
    4) <input type="number" aria-label="Offset"/> aka getByLabel('Offset')
    5) <input type="number" aria-label="Data Rate (hz)"/> aka getByLabel('Data Rate (hz)')
    6) <input type="number" aria-label="Phase (radians)"/> aka getByLabel('Phase (radians)')
    7) <input type="number" aria-label="Randomness"/> aka getByLabel('Randomness')
    8) <input type="number" aria-label="Loading Delay (ms)"/> aka getByLabel('Loading Delay (ms)')
    9) <input id="switchId" type="checkbox"/> aka getByRole('checkbox', { name: 'Include Infinity Values' })
    10) <input id="switchId" type="checkbox"/> aka getByRole('checkbox', { name: 'Exceed Float32 Limits' })
    ...

=========================== logs ===========================
waiting for locator('div:nth-child(5) .c-form-row__controls .form-control .field input')
============================================================
2.38% (2) 2 / 84 runs
failed over last 7 days
27.38% (23) 23 / 84 runs
flaked over last 7 days

⚠️ Flakes

📄   functional/plugins/imagery/exampleImagery.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Example Imagery Object Can use Mouse Wheel to zoom in and out of latest image
Retry 1Initial Attempt
0% (0) 0 / 66 runs
failed over last 7 days
28.79% (19) 19 / 66 runs
flaked over last 7 days

📄   functional/plugins/plot/overlayPlot.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Overlay Plot Limit lines persist when series is moved to another Y Axis and on refresh
Retry 1Initial Attempt
4.82% (4) 4 / 83 runs
failed over last 7 days
4.82% (4) 4 / 83 runs
flaked over last 7 days

View Detailed Build Results


Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (70f5ba9) 55.88% compared to head (b47ceff) 55.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7374      +/-   ##
==========================================
+ Coverage   55.88%   55.89%   +0.01%     
==========================================
  Files         659      659              
  Lines       26247    26278      +31     
  Branches     2550     2550              
==========================================
+ Hits        14667    14687      +20     
- Misses      10872    10882      +10     
- Partials      708      709       +1     
Flag Coverage Δ *Carryforward flag
e2e-full 41.24% <ø> (+0.09%) ⬆️ Carriedforward from ef64b0d
e2e-stable 58.97% <20.48%> (-0.09%) ⬇️
unit 48.91% <88.09%> (+<0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
example/imagery/plugin.js 79.66% <ø> (ø)
src/api/menu/MenuAPI.js 88.00% <100.00%> (-0.47%) ⬇️
src/api/overlays/Overlay.js 94.11% <ø> (-0.33%) ⬇️
src/api/overlays/OverlayAPI.js 86.66% <ø> (ø)
src/api/overlays/ProgressDialog.js 100.00% <100.00%> (ø)
src/exporters/CSVExporter.js 0.00% <ø> (ø)
src/exporters/ImageExporter.js 70.21% <ø> (ø)
src/exporters/JSONExporter.js 0.00% <ø> (ø)
...plugins/localStorage/LocalStorageObjectProvider.js 100.00% <100.00%> (ø)
...ins/notebook/actions/ExportNotebookAsTextAction.js 50.00% <ø> (ø)
... and 3 more

... 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 70f5ba9...b47ceff. Read the comment docs.

@ozyx ozyx force-pushed the mct7367-export-as-json-progress-bar branch from 655cb39 to 3a0c975 Compare January 17, 2024 19:15
@ozyx ozyx marked this pull request as ready for review January 17, 2024 21:21
await page.locator('div:nth-child(7) .c-form-row__controls .form-control .field input').fill('2');
await page.getByLabel('Period').fill('2');
await page.getByLabel('Amplitude').fill('6');
await page.getByLabel('Offset').fill('4');
Copy link
Collaborator

@unlikelyzero unlikelyzero Jan 17, 2024

Choose a reason for hiding this comment

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

lol what were we doing with the old locator pattern

@ozyx ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Jan 18, 2024
@github-actions github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jan 18, 2024
@unlikelyzero unlikelyzero modified the milestones: Target:3.3.0, Target:4.0.0 Jan 18, 2024
Copy link
Contributor

@scottbell scottbell left a comment

Choose a reason for hiding this comment

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

Good looks good. Example imagery now working:
Screenshot 2024-01-18 at 12 11 45 PM

and JSON exports for CouchDB now have a nifty progress bar:

Screen.Recording.2024-01-18.at.12.17.08.PM.mov

@scottbell scottbell merged commit 01434ff into master Jan 18, 2024
37 of 39 checks passed
@scottbell scottbell deleted the mct7367-export-as-json-progress-bar branch January 18, 2024 11:21
rukmini-bose pushed a commit that referenced this pull request Jan 19, 2024
* refactor(ExportAsJSONAction): use private methods

* refactor: remove unnecessary webpack alias

* refactor: lint

* fix: tests for `ExportAsJSONAction`

* test: stabilize `InspectorStylesSpec` tests

* docs: fix jsdocs

* chore: remove dead / redundant code

* refactor(LocalStorageObjectProvider): use `getItem()` and `setItem()`

* refactor(ExportAsJSONAction): use `Promise.all` where applicable

* refactor(MenuAPI): one-liner

* feat: add percentage ProgressBar to ExportAsJSONAction

* fix(ProgressBar.vue): v-if conditionals

* test(fix): update mockLocalStorage

* test: fix locators

* test: remove unneeded awaits

* fix: example imagery urls (moved after NASA wordpress migration)

* Revert "refactor(LocalStorageObjectProvider): use `getItem()` and `setItem()`"

This reverts commit 4f8403a.

* test(e2e): fix logPlot test

* Revert "Revert "refactor(LocalStorageObjectProvider): use `getItem()` and `setItem()`""

This reverts commit 0de6640.

* test(e2e): remove waitForNavigations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NASA moved the Example Imagery files Export as JSON needs an activity modal
3 participants