chore: make git message warnings remain dismissable #26812

jordanpowell88 · 2023-05-19T17:23:01Z

Closes Runs page Git messages should remain dismissed #26808

Additional details

Steps to test

How has the user experience changed?

If a user dismissed either the Git Repository Not Detected or No Runs Found For Your Branch Warnings it will no longer display

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

warrensplayer · 2023-05-19T17:42:09Z

packages/app/src/specs/banners/TrackedWarning.vue

+    ...TestingPreferences
+    ...SpecRunner_Preferences


These appear to just be copy/paste from other places. You don't need these fields in this component

warrensplayer · 2023-05-19T17:43:06Z

packages/app/src/specs/banners/TrackedWarning.cy.tsx

+  it('can mount', () => {
+    cy.mount(() => <div class="p-4"><TrackedWarning
+      data-testid="warning"
+      title={title}
+      message={message}
+      bannerId={bannerId}
+    /></div>)
+
+    cy.contains(title)
+    cy.get('[data-testid=warning]')
+    .should('contain.text', 'Hello!')
+    .and('contain.text', 'This is a markdown formatted message!')
+    .and('contain.text', `We're going to print out some console.log('cool code') and see how well it formats inside of our warning.`)
+  })


No need to retest the Warning functionality here. Just keep the test around the dismiss functionality.

warrensplayer · 2023-05-19T17:43:31Z

packages/app/src/specs/banners/TrackedWarning.cy.tsx

+  it('renders with a Learn more Link', () => {
+    // eslint-disable-next-line prefer-template
+    const messagePlusLink = message + '[Learn more](https://on.cypress.io/git-info)'
+
+    cy.mount(() => (<div class="p-4"><TrackedWarning
+      data-testid="warning"
+      title={title}
+      message={messagePlusLink}
+      bannerId={bannerId}
+    /></div>))
+
+    cy.contains('Learn more').should('have.attr', 'href', 'https://on.cypress.io/git-info')
+  })


Don't need this duplicate test from Warning

mike-plummer · 2023-05-19T19:36:28Z

cli/CHANGELOG.md

 - Add Git-related messages for the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) and [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when users aren't using Git or there are no recorded runs for the current branch. Fixes [#26680](https://github.com/cypress-io/cypress/issues/26680).

+- Never show one of the git-related message on the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) after it has been dismissed. Fixes[26808](https://github.com/cypress-io/cypress/issues/26808)


Suggested change

- Add Git-related messages for the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) and [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when users aren't using Git or there are no recorded runs for the current branch. Fixes [#26680](https://github.com/cypress-io/cypress/issues/26680).

- Never show one of the git-related message on the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) after it has been dismissed. Fixes[26808](https://github.com/cypress-io/cypress/issues/26808)

- Add Git-related messages for the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) and [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when users aren't using Git or there are no recorded runs for the current branch. Addresses [#26680](https://github.com/cypress-io/cypress/issues/26680) and [26808](https://github.com/cypress-io/cypress/issues/26808).

These are the same feature from the user's perspective, no need to have a separate changelog entry. And features should use "addresses" rather than "fixes".

mike-plummer · 2023-05-19T19:40:25Z

packages/app/src/specs/banners/TrackedWarning.vue

@@ -0,0 +1,62 @@
+<template>


Are we not able to use the existing TrackedBanner and pass a status of warning to get this same behavior?

☝️ bump @jordanpowell88 . Seems like this is the exact same as TrackedBanner minus the "shown" event being recorded. Couldn't we just add a prop to TrackedBanner to disable those?

@mike-plummer I suggested to Jordan to create a new component for this based on TrackedBanner, but looking at it more, I think you are correct. @jordanpowell88 Can you implement Mike's suggestion. If this does not make the release, then it is fine.

Just pushed changes which moves this logic to the TrackedBanner @mike-plummer @warrensplayer

mike-plummer · 2023-05-19T19:40:57Z

packages/app/src/specs/banners/TrackedBanner.vue

-    ...TestingPreferences
-    ...SpecRunner_Preferences


Why removed?

ah, I removed the wrong ones thanks for catching this

mike-plummer · 2023-05-19T19:41:02Z

packages/app/src/runs/RunsContainer.vue

        v-if="userProjectStatusStore.cloudStatusMatches('needsRecordedRun') && userProjectStatusStore.project.isUsingGit"
        :title="t('runs.empty.noRunsFoundForBranch')"
        :message="noRunsForBranchMessage"
+        banner-id="aci_052023_noRunsFoundForBranch"


Add new banner IDs to BannerIds map in packages/types/src/config.ts

Ah, didn't know these existed. Thank you

Import the values from the BannerId variables you added

Good idea. Done in 6e5dfc2

warrensplayer · 2023-05-22T13:13:52Z

packages/types/src/config.ts

@@ -48,6 +48,8 @@ export const BannerIds = {
  ACI_082022_CONNECT_PROJECT: 'aci_082022_connectProject',
  ACI_082022_RECORD: 'aci_082022_record',
  CT_052023_AVAILABLE: 'ct_052023_available',
+  ACI_052023_No_Runs_Found_For_Branch: 'aci_052023_noRunsFoundForBranch',


Nitpick, but the pattern is the variable names are all caps

Fixed in 6e5dfc2

warrensplayer · 2023-05-22T13:14:34Z

packages/app/src/runs/RunsContainer.vue

        v-if="!userProjectStatusStore.project.isUsingGit"
        :title="t('runs.empty.gitRepositoryNotDetected')"
        :message="t('runs.empty.ensureGitSetupCorrectly')"
+        banner-id="aci_052023_gitNotDetected"


Also import this value from BannerIds

warrensplayer · 2023-05-22T13:21:23Z

packages/app/src/specs/banners/TrackedWarning.cy.tsx

+
+    cy.get('@warning').should('be.visible')
+    cy.get(`[aria-label=${defaultMessages.components.alert.dismissAriaLabel}`).first().click()
+    cy.wrap(onUpdate).should('be.called')


This assertion just validates that the event is propagated out of the TrackedWarning. What should be tested is that the settings are saved to the server when the warning is dismissed. See an example for the TrackedBanner here: https://github.com/cypress-io/cypress/blob/develop/packages/app/src/specs/banners/TrackedBanner.cy.tsx#L38-L63.

The cy.stubMutationResolver can track when the GraphQL mutation is called.

Done in 6e5dfc2

cypress · 2023-05-22T15:54:24Z

27 flaky tests on run #47050 ↗︎

0	27307	1308	0	27

Details:

Merge branch 'develop' into jordanpowell88/git-message-remain-dismissable
Project: cypress	Commit: `42ae2e6710`
Status: Passed	Duration: 20:58 💡
Started: Jun 5, 2023 4:53 PM	Ended: Jun 5, 2023 5:14 PM

commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		`Output`

cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test		Artifacts
... > correctly returns currentRetry		`Output`
... > correctly returns currentRetry		`Output`
... > correctly returns currentRetry		`Output`

commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-chrome

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		`Output` `Video`

e2e/origin/commands/navigation.cy.ts • 1 flaky test • 5x-driver-chrome

View Output Video

Test		Artifacts
cy.origin navigation > #consoleProps > .go()		`Output` `Video`

create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > runs generated spec		`Output` `Screenshots` `Video`

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

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

warrensplayer · 2023-05-31T23:21:44Z

packages/app/src/specs/banners/TrackedBanner.cy.tsx

+    context('when eventData is undefined', () => {
+      it('should not record event', () => {
+        cy.mount({
+          render: () => <TrackedBanner bannerId="test-banner" hasBannerBeenShown={true} eventData={undefined} />,


To mimic the scenario being used in this PR, you would want to test with hasBannerBeenShown equal to false, correct?

Suggested change

render: () => <TrackedBanner bannerId="test-banner" hasBannerBeenShown={true} eventData={undefined} />,

render: () => <TrackedBanner bannerId="test-banner" hasBannerBeenShown={false} eventData={undefined} />,

warrensplayer

Should the RunsContainer.ct.tsx tests for these banners be updated to test the dismiss buttons?

lmiller1990 · 2023-06-05T05:28:20Z

packages/app/src/runs/RunsContainer.vue

      <Warning
        v-if="!online"
        :title="t('launchpadErrors.noInternet.header')"
        :message="t('launchpadErrors.noInternet.message')"
        :dismissible="false"
        class="mx-auto mb-[24px]"
      />
-      <Warning
+      <TrackedBanner


Nice re-use of this existing banner, forgot we had the TrackedBanner abstraction.

…able

Jordan updated the change log and addressed the original concerns.

cypress-bot · 2023-06-07T17:26:12Z

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.

warrensplayer reviewed May 19, 2023

View reviewed changes

jordanpowell88 marked this pull request as ready for review May 19, 2023 18:08

jordanpowell88 requested review from warrensplayer and a team May 19, 2023 18:08

mike-plummer previously requested changes May 19, 2023

View reviewed changes

jordanpowell88 force-pushed the jordanpowell88/git-message-remain-dismissable branch from fbe6624 to c510ac1 Compare May 19, 2023 19:56

warrensplayer reviewed May 22, 2023

View reviewed changes

jordanpowell88 requested review from mike-plummer and warrensplayer May 22, 2023 13:45

jordanpowell88 changed the title ~~feat: make git message warnings remain dismissable~~ chore: make git message warnings remain dismissable May 22, 2023

jordanpowell88 added 11 commits May 30, 2023 13:26

feat: make git message warnings remain dismissable

3ac4134

chore: update CHANGELOG

1aa18f7

chore: update CHANGELOG

05243e8

chore: remove unneeded code

d9e33d7

chore: update BannerId types

be04475

chore: fix queryies

6d5e0d8

chore: clean up PR

d65028b

chore: move TrackedWarning to frontend-shared

1afe51c

chore: update import

6c379de

ref: move TrackedWarning to TrackedBanner

8bef0fa

chore: udpate CHANGELOG

99e734f

jordanpowell88 force-pushed the jordanpowell88/git-message-remain-dismissable branch from 080336b to 99e734f Compare May 30, 2023 17:31

jordanpowell88 added 3 commits May 31, 2023 12:26

fix: update TrackedBanner to parse markdown message

bba108d

chore: set TrackedBanner default message prop

1b298f3

chore: update RunsContainer

42da149

warrensplayer reviewed May 31, 2023

View reviewed changes

warrensplayer reviewed Jun 1, 2023

View reviewed changes

chore: add missing tests and update alert type

89cee64

jordanpowell88 requested a review from warrensplayer June 1, 2023 15:52

lmiller1990 approved these changes Jun 5, 2023

View reviewed changes

warrensplayer approved these changes Jun 5, 2023

View reviewed changes

warrensplayer and others added 2 commits June 5, 2023 10:35

Merge branch 'develop' into jordanpowell88/git-message-remain-dismiss…

4a11719

…able

Merge branch 'develop' into jordanpowell88/git-message-remain-dismiss…

42ae2e6

…able

jordanpowell88 merged commit d77341e into develop Jun 5, 2023

jordanpowell88 deleted the jordanpowell88/git-message-remain-dismissable branch June 5, 2023 17:40

cypress-bot bot locked as resolved and limited conversation to collaborators Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: make git message warnings remain dismissable #26812

chore: make git message warnings remain dismissable #26812

jordanpowell88 commented May 19, 2023 •

edited

Loading

warrensplayer May 19, 2023

warrensplayer May 19, 2023

warrensplayer May 19, 2023

mike-plummer May 19, 2023

mike-plummer May 19, 2023

mike-plummer May 22, 2023

warrensplayer May 22, 2023

jordanpowell88 May 31, 2023

mike-plummer May 19, 2023

jordanpowell88 May 19, 2023

mike-plummer May 19, 2023

jordanpowell88 May 19, 2023

warrensplayer May 22, 2023

jordanpowell88 May 22, 2023

warrensplayer May 22, 2023

jordanpowell88 May 22, 2023

warrensplayer May 22, 2023

warrensplayer May 22, 2023

jordanpowell88 May 22, 2023

cypress bot commented May 22, 2023 •

edited

Loading

warrensplayer May 31, 2023

warrensplayer left a comment

lmiller1990 Jun 5, 2023

cypress-bot bot commented Jun 7, 2023

		- Add Git-related messages for the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) and [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when users aren't using Git or there are no recorded runs for the current branch. Fixes [#26680](https://github.com/cypress-io/cypress/issues/26680).

		- Never show one of the git-related message on the [Runs page](https://docs.cypress.io/guides/core-concepts/cypress-app#Runs) after it has been dismissed. Fixes[26808](https://github.com/cypress-io/cypress/issues/26808)

	render: () => <TrackedBanner bannerId="test-banner" hasBannerBeenShown={true} eventData={undefined} />,
	render: () => <TrackedBanner bannerId="test-banner" hasBannerBeenShown={false} eventData={undefined} />,

chore: make git message warnings remain dismissable #26812

chore: make git message warnings remain dismissable #26812

Conversation

jordanpowell88 commented May 19, 2023 • edited Loading

Additional details

Steps to test

How has the user experience changed?

PR Tasks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented May 22, 2023 • edited Loading

27 flaky tests on run #47050 ↗︎

Choose a reason for hiding this comment

warrensplayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress-bot bot commented Jun 7, 2023

jordanpowell88 commented May 19, 2023 •

edited

Loading

cypress bot commented May 22, 2023 •

edited

Loading