Skip to content
New issue

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

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

Already on GitHub? # to your account

Ensure CouchDB changes for plans trigger updates in the view #7099

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

shefalijoshi
Copy link
Contributor

@shefalijoshi shefalijoshi commented Sep 28, 2023

Closes #7098 7098

Describe your changes:

Listen to ALL changes for a plan since couchdb feed updates does not trigger a property only event. It triggers a catchall '*' event.
See: https://github.com/nasa/openmct/blob/fix-plan-updates/src/api/objects/ObjectAPI.js#L671

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?
  • Command line build passes?
  • Has this been smoke tested?
  • 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?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

…trigger a property only event. It triggers a catchall '*' event.
@deploysentinel
Copy link

deploysentinel bot commented Sep 28, 2023

Current Playwright Test Results Summary

✅ 14 Passing

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

(Last updated on 10/02/2023 11:25:24pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 9091358

Started: 10/02/2023 11:23:27pm UTC

View Detailed Build Results


Current Playwright Test Results Summary

✅ 141 Passing - ⚠️ 5 Flaky

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

(Last updated on 10/02/2023 11:25:24pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 9091358

Started: 10/02/2023 10:41:03pm UTC

⚠️ Flakes

📄   functional/plugins/timer/timer.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Timer Can perform actions on the Timer
Retry 1Initial Attempt
0% (0) 0 / 92 runs
failed over last 7 days
8.70% (8) 8 / 92 runs
flaked over last 7 days

📄   functional/plugins/telemetryTable/telemetryTable.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Telemetry Table unpauses and filters data when paused by button and user changes bounds
Retry 1Initial Attempt
0% (0) 0 / 92 runs
failed over last 7 days
23.91% (22) 22 / 92 runs
flaked over last 7 days

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

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 1Initial Attempt
7% (7) 7 / 100 runs
failed over last 7 days
25% (25) 25 / 100 runs
flaked over last 7 days

📄   functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds
Retry 1Initial Attempt
0.76% (1) 1 / 132 run
failed over last 7 days
55.30% (73) 73 / 132 runs
flaked over last 7 days

📄   functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Allows embeds to be deleted if page unlocked @addinit
Retry 1Initial Attempt
8.60% (8) 8 / 93 runs
failed over last 7 days
44.09% (41) 41 / 93 runs
flaked over last 7 days

View Detailed Build Results


@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #7099 (9091358) into master (6947c91) will decrease coverage by 0.20%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #7099      +/-   ##
==========================================
- Coverage   55.55%   55.35%   -0.20%     
==========================================
  Files         650      650              
  Lines       26090    26090              
  Branches     2549     2549              
==========================================
- Hits        14494    14442      -52     
- Misses      10890    10954      +64     
+ Partials      706      694      -12     
Flag Coverage Δ *Carryforward flag
e2e-full 41.93% <ø> (ø) Carriedforward from 6947c91
e2e-stable 57.17% <ø> (ø)
unit 48.79% <0.00%> (-0.31%) ⬇️

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

Files Coverage Δ
src/plugins/plan/components/Plan.vue 56.14% <0.00%> (+0.87%) ⬆️

... and 6 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 6947c91...9091358. Read the comment docs.

@shefalijoshi shefalijoshi removed the request for review from akhenry September 29, 2023 17:37
@ozyx ozyx self-requested a review September 29, 2023 20:18
@ozyx ozyx added this to the Target:3.1.0 milestone Oct 2, 2023
Copy link
Member

@ozyx ozyx left a comment

Choose a reason for hiding this comment

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

This doesn't seem to work?

I loaded up this branch in the VIPER testing environment, navigated to a VIPER plan and then made changes directly to the plan (changed an activity name) using the CouchDB utils interface. The plan did not update until I navved away / back again.

Is this not the correct way to test? How should this be tested?

Edit: It works, the issue was my local proxy setup.

@shefalijoshi
Copy link
Contributor Author

This doesn't seem to work?

I loaded up this branch in the VIPER testing environment, navigated to a VIPER plan and then made changes directly to the plan (changed an activity name) using the CouchDB utils interface. The plan did not update until I navved away / back again.

Is this not the correct way to test? How should this be tested?

It appears that the couchdb feed does not work as expected when using a proxy.
When deployed and tested on banner/dev, it appears to work fine. Please test there.

@shefalijoshi shefalijoshi requested a review from ozyx October 2, 2023 22:58
Copy link
Member

@ozyx ozyx left a comment

Choose a reason for hiding this comment

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

Tested and it works. The issue was my local proxy setup 🤦.
Great catch!

@ozyx ozyx enabled auto-merge (squash) October 2, 2023 23:20
@ozyx ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 2, 2023
@github-actions github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 2, 2023
@ozyx ozyx merged commit ede93d8 into master Oct 2, 2023
13 of 19 checks passed
@ozyx ozyx deleted the fix-plan-updates branch October 2, 2023 23:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plans are not updating automatically - browser refresh required
3 participants