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

chore: add GraphQL mutation for sending system notifications via Electron #26773

Merged
merged 14 commits into from
May 24, 2023

Conversation

astone123
Copy link
Contributor

@astone123 astone123 commented May 16, 2023

Additional details

This PR adds a new GraphQL mutation and Electron action that allows us to send a system notification to the OS using the Electron notifications API. We've manually tested these notifications on macOS, Linux, and Windows operating systems:

macOS

https://www.loom.com/share/18a187afab884957af3245508a0f3de3

Windows

OUTSTANDING ISSUE
There is an issue on Windows where when a user clicks the action to turn off notifications for Cypress, the Cypress app does not appear in the Windows notification settings. This means that users cannot re-enable notifications for the Cypress app once they have been disabled this way at the OS level.

I suspect that this has to do with the way that Cypress is installed. Most applications have a Windows installer that sets up shortcuts for the applications, but Cypress doesn't do this because it is launched from the command line. We are discussing how to work around this or find a solution.

Created #26786 to deal with this

https://www.loom.com/share/faf661068e6d4391b128aa4525cd3054

Ubuntu

electron_notifications_ubuntu_poc.mov

Steps to test

You can test the notifications using the GraphiQL GUI

  1. Run Cypress
  2. Open a project in a browser
  3. With Launchpad focused, click "Developer Tools" in the menu bar and select "GraphiQL"
  4. Write a mutation
mutation showNotification($title: String!, $body: String) {
  showSystemNotification(title: $title, body: $body)
}
  1. Add variables
{
  "title": "My notification",
  "body": "This is a test notification"
}
  1. Press the play button to execute the query
  2. Verify that Cypress sent a notification to your operating system (take action to allow notifications if needed)
  3. Verify that clicking on the notification focuses your active browser window (not Launchpad)

How has the user experience changed?

This should not change the user experience because this mutation is not being executed anywhere in the app.

PR Tasks

  • [na] Have tests been added/updated? I decided that it wouldn't be worth writing automated tests for this yet since it's not called yet. Once we use it somewhere, we can test it at that level instead
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@astone123 astone123 requested a review from a team May 16, 2023 22:46
@cypress
Copy link

cypress bot commented May 16, 2023

10 flaky tests on run #46461 ↗︎

0 4832 909 0 Flakiness 10

Details:

feedback
Project: cypress Commit: f48a0d2922
Status: Passed Duration: 14:48 💡
Started: May 17, 2023 11:13 PM Ended: May 17, 2023 11:28 PM
Flakiness  runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App: Runs > Runs - Create Project > when a project is created, injects new projectId into the config file, and sends expected UTM params Output Screenshots Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video
Flakiness  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found Output Screenshots Video
Flakiness  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution Output Screenshots Video
Flakiness  commands/net_stubbing.cy.ts • 2 flaky tests • 5x-driver-webkit

View Output Video

Test Artifacts
... > with `times` > only uses each handler N times Output Video
... > stops waiting when an xhr request is canceled Output Video

The first 5 flaky specs are shown, see all 7 specs in Cypress Cloud.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@mike-plummer mike-plummer requested a review from a team May 17, 2023 21:18
@mike-plummer
Copy link
Contributor

@astone123 I was poking through the Electron notification docs and came across two things:

  1. I noticed the Notification.isSupported() flag - any value in exposing this through, or checking it prior to attempting to show a Notification?
  2. There's an additional close event that might be nice to allow consumers to register for (so they could know both when a notification was acted on (click) or dismissed (close)....but according to the docs it doesn't trigger in all circumstances so maybe not worth it. A partial event is sometimes worse than none at all

@astone123
Copy link
Contributor Author

@mike-plummer

  1. Yeah I saw this - I can't think of a scenario where this would return false, maybe if the user was on a super old OS. Not sure if Cypress would even run correctly on an OS that doesn't support notifications. Might be worth looking into in the future though.
  2. We can definitely add this later if needed, but yeah that stood out to me in the docs. I wonder under what circumstances it wouldn't trigger the close event? Seems kind of vague. As of right now I don't think we have a use for it in this initiative

Comment on lines 112 to 116
const defaultOnClick = async () => {
await this.ctx.actions.browser.focusActiveBrowserWindow()
})
}

const clickHandler = onClick || defaultOnClick
Copy link
Contributor

Choose a reason for hiding this comment

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

/nit: Could avoid building this function if an onClick was provided to save a processor cycle:

const clickHandler = onClick || (async () => {
  await this.ctx.actions.browser.focusActiveBrowserWindow()
})

@mike-plummer mike-plummer requested a review from a team May 19, 2023 15:30
@astone123 astone123 merged commit b1f699a into develop May 24, 2023
@astone123 astone123 deleted the notification-testing branch May 24, 2023 16:10
mjhenkes added a commit that referenced this pull request Jun 2, 2023
* chore: add Nx Cloud (#26712)

* chore: add empty nx.json [run ci]

* chore: add nx cloud runner [run ci]

* chore: add nx-cloud dep [run ci]

* chore: update local nx cloud accessToken to be read-only

* feat: update git related messages for runs and debug (#26758)

* feat(app): update DebugError copy

* feat: set isUsingGit project flag and consume in DebugContainer

* feat(app): update not using git icon for DebugError

* feat(app): displays alert on runs page when not using git

* feat(app): add component for when no runs for current branch

* feat(app): add warning for no runs for branch on runs container

* chore: add feat to CHANGELOG

* chore: remove logged status value

* chore: resolve import from merge conflict

* test(app): stub branch name for e2e runs spec

* chore: add line break in changelog entry

* chore: cleanup PR

* chore: fix i18n import for DebugBranchError

* chore: add utm and update Warning links to inline

* chore: capitalize Git in i18n

* ref: warning liink

* test: add i18n to tests

* test(app): change utm_source assertions

* chore: cleanup pr

* chore: remove unused prop

* test(app): remove no git warning when moving to runs page in e2e

* chore: change template logic

* chore: remove duplicate RUNS_TAB_MEDIUM const

* Changed Debug test assertion and reordered new components for Debug

---------

Co-authored-by: Stokes Player <stokes.player@gmail.com>

* chore: rename video processing events to capture/compress (#26800)

* chore: change processing nomenclature to compressing when printing the run.

* chore: rename 'capturing of' to 'capturing'

* chore: rename upload results to upload screenshots & videos (#26811)

* chore: rename upload results to upload screenshots & videos

* run ci

* chore: capture versions of relevant dependencies with `x-dependencies` header (#26814)

* chore: update changlelog script to handle revert pr ref (#26801)

* fix: Correct typescript scaffold dependency (#26815)

* fix: correct typescript scaffold dependency (#26204)

* add changelog

* Update change log for PR comment

Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>

---------

Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>

* chore: 12.13.0 prep (#26833)

* chore: 12.13.0 release (#26834)

* chore: 12.13.0 changelog fix

* remove pending, bump version

* fix typo

* chore: release @cypress/vite-plugin-cypress-esm-v1.0.1

[skip ci]

* chore: Implement runSpec mutation (#26782)

* chore: replace gitter badge with discord on readme (#26771)

* chore: add GraphQL mutation for sending system notifications via Electron (#26773)

* fix: upgrade typescript from 4.7.4 to 4.9.5 (#26826)

Snyk has created this PR to upgrade typescript from 4.7.4 to 4.9.5.

See this package in npm:


See this project in Snyk:
https://app.snyk.io/org/cypress-opensource/project/d5b36925-e6ee-455d-9649-6560a9aca413?utm_source=github&utm_medium=referral&page=upgrade-pr

* test: fix 2 broken tests for Windows (#26854)

* Update stale_issues_and_pr_cleanup.yml

Upped the number of operations per run.  Have been manually doing that so this job can get through all the issues in the repo with no problems.

* chore(dep): [Snyk] Upgrade vite from 2.9.13 to 2.9.15 (#26830)

Co-authored-by: Ben M <benm@cypress.io>

* Update triage_add_to_project.yml

* chore: fix minor background color styling in debug results component (#26887)

* Update stale_issues_and_pr_cleanup.yml

stalebot was incorrectly configured to run in debug mode.  I have updated the default to run in normal mode when running scheduled

* chore: Deprecate @cypress/xpath package (#26893)

* chore: add telemetry realworld app (#26896)

* chore: capture telemetry for realworld app maybe

* idk what i was doing

* setup record key and telemetry

* testing

* override project id

* some times we just need a little context.

* Adding tests

* Adding comment

* fix yarn lock

* Trying this....

* fix tests?

---------

Co-authored-by: Jordan <jordan@jpdesigning.com>
Co-authored-by: Stokes Player <stokes.player@gmail.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Co-authored-by: Adam Stone-Lord <adams@cypress.io>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Dave Kasper <dave.m.kasper@gmail.com>
Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>
Co-authored-by: Ely Lucas <ely@meta-tek.net>
Co-authored-by: Snyk bot <snyk-bot@snyk.io>
Co-authored-by: Stokes Player <stokes@cypress.io>
Co-authored-by: Ben M <benm@cypress.io>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
mjhenkes added a commit that referenced this pull request Jun 2, 2023
* chore: add Nx Cloud (#26712)

* chore: add empty nx.json [run ci]

* chore: add nx cloud runner [run ci]

* chore: add nx-cloud dep [run ci]

* chore: update local nx cloud accessToken to be read-only

* feat: update git related messages for runs and debug (#26758)

* feat(app): update DebugError copy

* feat: set isUsingGit project flag and consume in DebugContainer

* feat(app): update not using git icon for DebugError

* feat(app): displays alert on runs page when not using git

* feat(app): add component for when no runs for current branch

* feat(app): add warning for no runs for branch on runs container

* chore: add feat to CHANGELOG

* chore: remove logged status value

* chore: resolve import from merge conflict

* test(app): stub branch name for e2e runs spec

* chore: add line break in changelog entry

* chore: cleanup PR

* chore: fix i18n import for DebugBranchError

* chore: add utm and update Warning links to inline

* chore: capitalize Git in i18n

* ref: warning liink

* test: add i18n to tests

* test(app): change utm_source assertions

* chore: cleanup pr

* chore: remove unused prop

* test(app): remove no git warning when moving to runs page in e2e

* chore: change template logic

* chore: remove duplicate RUNS_TAB_MEDIUM const

* Changed Debug test assertion and reordered new components for Debug

---------

Co-authored-by: Stokes Player <stokes.player@gmail.com>

* chore: rename video processing events to capture/compress (#26800)

* chore: change processing nomenclature to compressing when printing the run.

* chore: rename 'capturing of' to 'capturing'

* chore: rename upload results to upload screenshots & videos (#26811)

* chore: rename upload results to upload screenshots & videos

* run ci

* chore: capture versions of relevant dependencies with `x-dependencies` header (#26814)

* chore: update changlelog script to handle revert pr ref (#26801)

* fix: Correct typescript scaffold dependency (#26815)

* fix: correct typescript scaffold dependency (#26204)

* add changelog

* Update change log for PR comment

Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>

---------

Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>

* chore: 12.13.0 prep (#26833)

* chore: 12.13.0 release (#26834)

* chore: 12.13.0 changelog fix

* remove pending, bump version

* fix typo

* chore: release @cypress/vite-plugin-cypress-esm-v1.0.1

[skip ci]

* chore: Implement runSpec mutation (#26782)

* chore: replace gitter badge with discord on readme (#26771)

* chore: add GraphQL mutation for sending system notifications via Electron (#26773)

* fix: upgrade typescript from 4.7.4 to 4.9.5 (#26826)

Snyk has created this PR to upgrade typescript from 4.7.4 to 4.9.5.

See this package in npm:


See this project in Snyk:
https://app.snyk.io/org/cypress-opensource/project/d5b36925-e6ee-455d-9649-6560a9aca413?utm_source=github&utm_medium=referral&page=upgrade-pr

* test: fix 2 broken tests for Windows (#26854)

* Update stale_issues_and_pr_cleanup.yml

Upped the number of operations per run.  Have been manually doing that so this job can get through all the issues in the repo with no problems.

* chore(dep): [Snyk] Upgrade vite from 2.9.13 to 2.9.15 (#26830)

Co-authored-by: Ben M <benm@cypress.io>

* Update triage_add_to_project.yml

* chore: fix minor background color styling in debug results component (#26887)

* Update stale_issues_and_pr_cleanup.yml

stalebot was incorrectly configured to run in debug mode.  I have updated the default to run in normal mode when running scheduled

* chore: Deprecate @cypress/xpath package (#26893)

* chore: add telemetry realworld app (#26896)

* chore: capture telemetry for realworld app maybe

* idk what i was doing

* setup record key and telemetry

* testing

* override project id

* some times we just need a little context.

* Adding tests

* Adding comment

* chore(deps): update dependency find-process to v1.4.7 🌟 (#26906)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

* chore(deps): update dependency firefox-profile to v4.3.2 🌟 (#26912)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

---------

Co-authored-by: Jordan <jordan@jpdesigning.com>
Co-authored-by: Stokes Player <stokes.player@gmail.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Co-authored-by: Adam Stone-Lord <adams@cypress.io>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Dave Kasper <dave.m.kasper@gmail.com>
Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>
Co-authored-by: Ely Lucas <ely@meta-tek.net>
Co-authored-by: Snyk bot <snyk-bot@snyk.io>
Co-authored-by: Stokes Player <stokes@cypress.io>
Co-authored-by: Ben M <benm@cypress.io>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 7, 2023

Released in 12.14.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.14.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement system notifications infrastructure
3 participants