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

jordanpowell88 · 2023-05-15T15:43:29Z

Closes ACI - Updated Git related messages for Runs and Debug page #26680

Additional details

This PR adds messaging for users who are either NOT using git or their are NO recorded runs for their current branch on the debug and runs page.

Steps to test

yarn workspace @packages/app dev

How has the user experience changed?

DEBUG PAGE

RUNS PAGE

PR Tasks

[X ] 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?

cypress · 2023-05-15T16:22:53Z

30 flaky tests on run #46563 ↗︎

0	27302	1308	0	30

Details:

Changed Debug test assertion and reordered new components for Debug
Project: cypress	Commit: `ce63656228`
Status: Passed	Duration: 20:12 💡
Started: May 19, 2023 12:40 PM	Ended: May 19, 2023 1:01 PM

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

View Output Video

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

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

View Output Video

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

create-from-component.cy.ts • 2 flaky tests • app-e2e

View Output Video

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

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`

cypress-in-cypress.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress in Cypress > scales the AUT correctly in component		`Output` `Screenshots` `Video`

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

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

cli/CHANGELOG.md

packages/app/src/debug/DebugContainer.vue

mike-plummer · 2023-05-16T14:05:26Z

packages/app/src/debug/empty/DebugBranchError.vue

+    :title="t('debugPage.emptyStates.noRunsFoundForBranch')"
+    :description="t('debugPage.emptyStates.noRunsForBranchMessage')"
+    :icon="IconTechnologyCommandLineError"
+    help-link-href="https://on.cypress.io/git-info"


No UTM params needed here?

No UTM params needed here?

I don't see anything about UTM params in the requirements. @warrensplayer does this need added?

Asking about this...

Yes, the links should have UTM params. The requirements in the issue have been updated.

mike-plummer · 2023-05-16T14:07:13Z

packages/app/src/debug/empty/DebugBranchError.cy.tsx

+describe('<DebugBranchError />', () => {
+  it('can mount', () => {
+    cy.mount(<DebugBranchError />)
+    cy.contains('No runs found for your branch')


Could just reference the i18n bundle in this test so we don't inline a bunch of text:

import defaultMessages from '@packages/frontend-shared/src/locales/en-US.json' ... cy.contains(defaultMessages.debugPage.emptyStates.noRunsFoundForBranch) cy.contains(defaultMessages.debugPage.emptyStates.noRunsForBranchMessage)

Yes, added in 28d8f35

packages/app/src/debug/empty/DebugEmptyStates.cy.tsx

packages/app/src/debug/empty/DebugEmptyView.vue

mike-plummer · 2023-05-16T14:11:01Z

packages/frontend-shared/src/locales/en-US.json

      "learnAboutRecordingSrText": "about recording a run to Cypress Cloud",
      "learnAboutDebuggingSrText": "about debugging CI failures in Cypress",
      "learnAboutProjectSetupSrText": "about project setup in Cypress",
+      "noRunsFoundForBranch": "No runs found for your branch",
+      "noRunsForBranchMessage": "Cypress uses Git to show runs for your branch. Ensure that version control is properly configured and that you are sending Git information to Cypress Cloud.",


Why uppercase Git here, but lowercase in message above?

This is the way it was in the ticket. @warrensplayer should the requirements be changed?

"Git" should be capitalized. I will get the requirements and Figma updated.

Screenshot in issue has been updated

mike-plummer · 2023-05-16T14:15:40Z

packages/frontend-shared/src/warning/Warning.vue

@@ -64,6 +66,11 @@ let message = computed(() => {
    return [props.message, `        ${ props.details }`].join('\n\n')
  }

+  if (props.helpLinkHref) {
+    // eslint-disable-next-line prefer-template
+    return props.message + ` <span class="ml-[4px]"><a href="${props.helpLinkHref}" class="text-indigo-500 hocus-link-default">${t('links.learnMoreButton')}</a></span>`


Is this just trying to shove a link into the message so it gets processed as markdown or whatever for the v-html? Why not just add a conditional ExternalLink in the template?

<div ref="markdownTarget" class="warning-markdown" v-html="markdown" /> <ExternalLink v-if="helpLinkHref" ..... >

This needs to be added to the props.message so that the link display inline the p copy.

Are we able to tack this on using markdown format rather than inline HTML? Would this get processed correctly by useMarkdown?

const learnMoreLabel = t('links.learnMoreButton') return `${props.message} [${learnMoreLabel}](${props.helpLinkHref})`

Ahhh, yes. Now i'm following you. Yes it is implemented in 27100e3

mike-plummer · 2023-05-16T14:18:38Z

cli/package.json

@@ -125,42 +125,42 @@
      "require": "./index.js"


The weirdness in this file may have been fixed in this PR? Maybe try pulling latest from develop and re-run the yarn install

Rebased in 28d8f35

This file is still showing diffs that are not related to this PR. You might just have to manually revert them by comparing the file to develop and syncing the changes

mike-plummer · 2023-05-17T21:07:06Z

@jordanpowell88 Looks like there's still some legitimate test failures here

jordanpowell88 · 2023-05-17T21:48:55Z

@jordanpowell88 Looks like there's still some legitimate test failures here

They should be resolved now. It was erroring because of UTM params in CT do not run right apparently. I updated the test to reflect this change in e33d420

warrensplayer

Almost done reviewing, but here are a few things to address.

warrensplayer · 2023-05-17T19:59:33Z

cli/package.json

@@ -125,42 +125,42 @@
      "require": "./index.js"


This file is still showing diffs that are not related to this PR. You might just have to manually revert them by comparing the file to develop and syncing the changes

warrensplayer · 2023-05-18T11:48:20Z

packages/app/src/runs/RunsContainer.vue

+  url: 'https://on.cypress.io/git-info',
+  params: {
+    utm_source: getUtmSource(),
+    utm_medium: DEBUG_TAB_MEDIUM,


This should be 'Runs Tab', not 'Debug Tab'

Fixed in e1f23d0

warrensplayer · 2023-05-18T11:49:55Z

packages/app/src/runs/RunsContainer.vue

    <RunsConnect
      v-if="!currentProject?.projectId || !cloudViewer?.id"
      :campaign="!cloudViewer?.id ? RUNS_PROMO_CAMPAIGNS.login : RUNS_PROMO_CAMPAIGNS.connectProject"
    />
+    <Warning
+      v-else-if="userProjectStatusStore.cloudStatusMatches('needsRecordedRun') && userProjectStatusStore.project.isUsingGit"
+      :title="t('debugPage.emptyStates.noRunsFoundForBranch')"


The Runs page copy is different than the Debug page. This component needs different i18n messaging instead of reusing the Debug messaging.

Ahhh, I didn't see this subtle difference, Fixed in e1f23d0

warrensplayer · 2023-05-18T11:50:01Z

packages/app/src/runs/RunsContainer.vue

@@ -32,6 +40,11 @@
        :dismissible="false"
        class="mx-auto mb-[24px]"
      />
+      <Warning
+        v-if="!userProjectStatusStore.project.isUsingGit"


The Runs page copy is different than the Debug page. This component needs different i18n messaging instead of reusing the Debug messaging.

Also fixed in e1f23d0

warrensplayer · 2023-05-18T11:51:58Z

packages/app/src/runs/RunsContainer.vue

+import { getUrlWithParams } from '@packages/frontend-shared/src/utils/getUrlWithParams'
+import { getUtmSource } from '@packages/frontend-shared/src/utils/getUtmSource'
+import { DEBUG_TAB_MEDIUM } from '../debug/utils/constants'


I think this is okay for now, but I find when I start adding several new imports to a component that already has a lot of imports that it might be a sign to make a separate component. Mainly just to keep this parent component to a manageable level of dependencies. Just something to consider for the future.

warrensplayer · 2023-05-18T12:05:55Z

packages/frontend-shared/src/warning/Warning.vue

+  if (props.helpLinkHref) {
+    const learnMoreLabel = t('links.learnMoreButton')
+
+    return `${props.message} [${learnMoreLabel}](${props.helpLinkHref})`
+  }
+


This is adding a very specific prop to this generic component. Since this is just formatting markdown in the message, you can just add the Learn More link to the message already formatted as Markdown. No need to modify this component.

Yep, now that this is working using the markdown via @mike-plummer suggestion we can remove this. Fixed in e1f23d0

warrensplayer · 2023-05-18T12:08:08Z

packages/app/src/debug/empty/DebugEmptyView.vue


 const { t } = useI18n()

 const props = defineProps<{
  title: string
  description?: string
+  icon?: FunctionalComponent<SVGAttributes, {}>
+  exampleTestName?: string


This prop is not used

warrensplayer · 2023-05-18T15:42:22Z

packages/app/src/debug/utils/constants.ts

@@ -1,5 +1,7 @@
 export const DEBUG_TAB_MEDIUM = 'Debug Tab'

+export const RUNS_TAB_MEDIUM = 'Runs Tab'


This can be removed

warrensplayer

@jordanpowell88 Looks good to go now. I did push a small tweak to the order of the components in the conditional list on DebugContainer to make sure the missing Git message only showed after they logged in. That was not specified in the requirements, but it encourages the user to log in first before worrying about Git.

mike-plummer · 2023-05-19T15:03:55Z

packages/app/src/runs/RunsContainer.vue

+  const learnMoreLink = getUrlWithParams({
+    url: 'https://on.cypress.io/git-info',
+    params: {
+      utm_source: getUtmSource(),


/nit: I think getUrlWithParams automatically applies a utm_source value which ends up overwriting any you supply

cypress-bot · 2023-05-23T23:32:59Z

Released in 12.13.0.

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

jordanpowell88 changed the title ~~Jordanpowell88/update git messages~~ feat: update git related messages for runs and debug page May 15, 2023

jordanpowell88 force-pushed the jordanpowell88/update-git-messages branch from d7bddf5 to 10341b9 Compare May 15, 2023 18:17

jordanpowell88 requested review from warrensplayer and astone123 May 16, 2023 13:51

jordanpowell88 changed the title ~~feat: update git related messages for runs and debug page~~ feat: update git related messages for runs and debug May 16, 2023

mike-plummer requested changes May 16, 2023

View reviewed changes

jordanpowell88 force-pushed the jordanpowell88/update-git-messages branch from 19e7466 to 28d8f35 Compare May 16, 2023 15:11

jordanpowell88 requested a review from mike-plummer May 17, 2023 19:24

warrensplayer requested changes May 18, 2023

View reviewed changes

jordanpowell88 added 18 commits May 18, 2023 08:54

feat(app): update DebugError copy

af7d0d6

feat: set isUsingGit project flag and consume in DebugContainer

a3e7b60

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

169a9c6

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

e7a2c6b

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

2fccb00

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

a7bc3f5

chore: add feat to CHANGELOG

16c42b4

chore: remove logged status value

dc4cfd1

chore: resolve import from merge conflict

d0de4a1

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

cb503f9

chore: add line break in changelog entry

345f875

chore: cleanup PR

ad35a26

chore: fix i18n import for DebugBranchError

07340a7

chore: add utm and update Warning links to inline

106bd75

chore: capitalize Git in i18n

12fb5c4

ref: warning liink

a76ddc5

test: add i18n to tests

b117c00

test(app): change utm_source assertions

eee6aec

chore: cleanup pr

e1f23d0

jordanpowell88 force-pushed the jordanpowell88/update-git-messages branch from 3014ce0 to e1f23d0 Compare May 18, 2023 13:46

jordanpowell88 added 2 commits May 18, 2023 09:47

chore: remove unused prop

ebd7176

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

89177e5

jordanpowell88 requested a review from warrensplayer May 18, 2023 16:14

chore: change template logic

0bb8ce3

warrensplayer reviewed May 18, 2023

View reviewed changes

jordanpowell88 added 2 commits May 18, 2023 13:09

chore: remove duplicate RUNS_TAB_MEDIUM const

c6f5bb9

Merge branch 'develop' into jordanpowell88/update-git-messages

95a2623

jordanpowell88 requested a review from warrensplayer May 18, 2023 21:09

Changed Debug test assertion and reordered new components for Debug

ce63656

warrensplayer approved these changes May 19, 2023

View reviewed changes

mike-plummer approved these changes May 19, 2023

View reviewed changes

jordanpowell88 merged commit a0efc77 into develop May 19, 2023

jordanpowell88 deleted the jordanpowell88/update-git-messages branch May 19, 2023 15:35

cypress-bot bot locked as resolved and limited conversation to collaborators May 23, 2023

		@@ -1,5 +1,7 @@
		export const DEBUG_TAB_MEDIUM = 'Debug Tab'

		export const RUNS_TAB_MEDIUM = 'Runs Tab'

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

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

Conversation

jordanpowell88 commented May 15, 2023

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented May 15, 2023 • edited Loading

30 flaky tests on run #46563 ↗︎

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

mike-plummer commented May 17, 2023

jordanpowell88 commented May 17, 2023

warrensplayer left a comment • edited Loading

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

warrensplayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress-bot bot commented May 23, 2023

cypress bot commented May 15, 2023 •

edited

Loading

warrensplayer left a comment •

edited

Loading