Creation Quote Metric #55

mtakeda · 2023-07-14T13:53:57Z

What problem is this solving?

Sending quotation created metrics to analytics in order to have more business data to take decisions and analyze user's behavior.

How to test it?

Create a quote in https://takeda--b2bstore005.myvtex.com/deals
After one hour (maximum), data should appear in Analytics Redshift, into table "vtex"."schemaless"."b2b_suite_buyerorg_data_raw"

Workspace

Screenshots or example usage:

No changes in layout. Current quotation create workflow kept as is.

Redshit query and result example:

SELECT
    payload.kind,
    payload.account,
    payload.fields.quote_id,
    payload.fields.quote_reference_name,
    payload.fields.creation_date,
    payload.fields.send_to_sales_rep,
    payload.fields.org_id,
    payload.fields.cost_center_id,
    payload.fields.cost_center_name,
    payload.fields.buy_org_id,
    payload.fields.buy_org_name,
    payload.fields.member_id,
    payload.fields.member_email,
    payload.fields.role
FROM
    "vtex"."schemaless"."b2b_suite_buyerorg_data_raw"
WHERE payload.name = 'b2b-suite-buyerorg-data'
order by ingestion_time desc

type:

type Metric = {
  name: 'b2b-suite-buyerorg-data'
  kind: 'create-quote-ui-event'
  description: 'Create Quotation Action - UI'
  account: string
}

type QuoteFieldsMetric = {
  org_id: string
  cost_center_id: string
  cost_center_name: string
  buy_org_id: string
  buy_org_name: string
  member_id: string
  member_email: string
  role: string
  creation_date: string
  quote_id: string
  quote_reference_name: string
  send_to_sales_rep: boolean
}

type QuoteMetric = Metric & { fields: QuoteFieldsMetric }

Describe alternatives you've considered, if any.

NA

Related to / Depends on

NA

How does this PR make you feel?

…queries extraction demand it

vtex-io-ci-cd · 2023-07-14T13:53:59Z

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

Patch (backwards-compatible bug fixes)
Minor (backwards-compatible functionality)
Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

No thanks, I would rather do it manually 😞

vtex-io-docs-bot · 2023-07-14T13:54:12Z

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

There's nothing new to document 🤔
I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

I just updated 🎉🎉

github-actions · 2023-07-14T13:55:00Z

	Messages
📖	❤️ Thanks!
📖	🎉 PR additions = `299`, PR deletions = `5`

Generated by 🚫 dangerJS against 4bff5c2

docs/README.md

mairatma · 2023-07-14T17:01:10Z

docs/README.md

+---
+


I think you forgot these lines

mairatma · 2023-07-14T17:08:31Z

react/components/QuoteDetails/QuoteDetails.tsx

+  const userEmail = sessionResponse?.namespaces?.profile?.email?.value
+  const { data: userData } = useQuery(CHECK_PERMISSIONS, {
+    variables: { email: userEmail },
+  })
+
+  const costCenterId: string = userData?.getUserByEmail?.[0]?.costId
+  const { data: costCenterData } = useQuery(GET_COSTCENTER_BY_ID, {
+    variables: { id: costCenterId },
+  })
+
+  const orgId: string = userData?.getUserByEmail?.[0]?.orgId
+  const { data: organizationData } = useQuery(GET_ORGANIZATION_BY_ID, {
+    variables: { id: orgId },
+  })


This will make us fetch all that data every time this page is rendered, even if we end up not needing it.

It seems like we only ever need this when sending the metric, right? So it's best if we only do this data fetching at that time, within the sendMetric function.

In that case you won't be doing the fetching using hooks (hooks are these functions with names starting with use). React hooks have logic behind the scenes that cause the screen to be rerendered when the data is loading, and then again when it arrives. For your use case we should do everything behind the scenes and without causing rerenders. If these data fetches were not graphql calls you could just use axios for that, for example. In graphql's case you can use a simple library like graphql-request instead.

mairatma · 2023-07-14T17:10:30Z

react/components/QuoteDetails/QuoteDetails.tsx

+  const userEmail = sessionResponse?.namespaces?.profile?.email?.value
+  const { data: userData } = useQuery(CHECK_PERMISSIONS, {
+    variables: { email: userEmail },
+  })
+
+  const costCenterId: string = userData?.getUserByEmail?.[0]?.costId
+  const { data: costCenterData } = useQuery(GET_COSTCENTER_BY_ID, {
+    variables: { id: costCenterId },
+  })
+
+  const orgId: string = userData?.getUserByEmail?.[0]?.orgId
+  const { data: organizationData } = useQuery(GET_ORGANIZATION_BY_ID, {
+    variables: { id: orgId },


One of the advantages of graphql is that you don't need to do separate fetches to get information. Since you need all three of these to send the metric, you can build a single graphql query joining them all together in a single request. Also, it's probably better to build a new query for this use case anyway, since you probably don't need the exact same data that those existing queries were asking for. You can drop the unused fields and join the queries together, resulting in a better API request.

mairatma · 2023-07-14T17:12:55Z

react/components/QuoteDetails/QuoteDetails.tsx

+            userData: userData?.getUserByEmail?.[0],
+            costCenterName: costCenterData?.getCostCenterById?.name,
+            buyOrgName: organizationData?.getOrganizationById?.name,


I'm not 100% sure, but it's possible that all this data is already present in the session or segment cookie, so it would be good to double check before making the changes I suggested to the data fetching.

mairatma · 2023-07-14T17:14:38Z

react/utils/metrics.ts

+  try {
+    const metric = buildQuoteMetric(sendToSalesRep, metricsParam)
+
+    axios.post(ANALYTICS_URL, metric)


It's better to have an await call here, otherwise you won't catch errors correctly

Suggested change

axios.post(ANALYTICS_URL, metric)

await axios.post(ANALYTICS_URL, metric)

In this case, I'm not sure I need to catch errors. The idea of not having an await is not having to wait for the response from analytics API and not delay a bit more the response for users.

I agree that it's not very important for you to catch errors here, but in that case you should remove the try/catch in this function. Without the await it won't work well if the request actually fails.

If you want to keep catching the errors here you can add the await. That won't really change the behavior of the call, because you don't await the sendMetric call itself, so it will still run in the background without affecting the rendering thread in any way.

mairatma · 2023-07-14T17:14:59Z

react/utils/metrics.ts

+
+    axios.post(ANALYTICS_URL, metric)
+  } catch (error) {
+    console.error('Unable to log metrics', error)


I think it's best to make this a warn, since its a background call that doesn't (and shouldn't) break the page.

mairatma

Thanks Mauro! Just smaller suggestions now.

mairatma · 2023-07-19T11:14:09Z

react/utils/metrics.ts

+      getUserByEmail(email: $email) @context(provider: "vtex.storefront-permissions") {
+      costId
+    }


Just a small indentation fix:

Suggested change

getUserByEmail(email: $email) @context(provider: "vtex.storefront-permissions") {

costId

}

getUserByEmail(email: $email) @context(provider: "vtex.storefront-permissions") {

costId

}

mairatma · 2023-07-19T11:19:27Z

react/utils/metrics.ts

+      buy_org_name: metricsData.organizationName,
+      cost_center_id: metricsData.costId,
+      cost_center_name: metricsData.costCenterName,
+      member_id: namespaces?.profile?.id?.value,


In line 98 the code is assuming that namespaces.profile.email.value is always set, but here you're handling the case where namespaces and profile might not be set. If that's a possibility we need to also add this to lines 97 and 98, skipping the whole metric process in case the values are not set (since those are required).

Instead of skipping the while metric process, I'll check the namespace, profile, and email, and let it save null values. I believe it's better to save the metric with the data we have than not save it at all

I agree, if the metric is still useful, let's save it!

mairatma · 2023-07-19T11:22:03Z

react/utils/metrics.ts

+  const { data } = (
+    await axios.post(GRAPHQL_URL(accountName, workspace), query)
+  ).data
+


When a GraphQL request fails, the response status is often 200 anyway, depending on the type of the error (mainly because there can be partial errors). So there would be no exception thrown here, but data might be undefined, or it might only have part of the information you requested. In those cases the response will contain a field called errors, so you can check for that here.

Suggested change

const { data } = (

await axios.post(GRAPHQL_URL(accountName, workspace), query)

).data

const { data, errors } = (

await axios.post(GRAPHQL_URL(accountName, workspace), query)

).data

More information on graphql errors here.

mairatma · 2023-07-19T11:25:16Z

react/utils/metrics.ts

+  ).data
+
+  const quoteResult = data.getQuote as Omit<QuoteMetricsData, 'costId'>
+  const costId = data.getUserByEmail?.[0].costId as string


You're using an optional operator here, so you're assuming that data.getUserByEmail might not be set, right? If that's a possibility then you shouldn't cast to string here, as it'll make costId's type not consider the use case where it will be undefined. Is it ok if costId isn't set? If it isn't it's better to bail early here, and if it's ok then you should let it be string | undefined.

react/components/QuoteDetails/QuoteDetails.tsx

mairatma

Lgtm, thanks! 👏

mairatma · 2023-07-24T12:41:28Z

@mtakeda don't forget to select the version bump within vtex-io-ci-cd bot's message at the further above in this PR, or that check will keep failing.

Use Quote Metric

mairatma · 2023-08-03T11:41:21Z

manifest.json

@@ -1,7 +1,7 @@
 {
  "vendor": "vtex",
  "name": "b2b-quotes",
-  "version": "1.5.5",
+  "version": "1.5.6",


This shouldn't be changed here in the PR. It will be changed automatically by the vtex io bot once this PR is merged. You should revert this change, and just select the right type of version bump in the question post from vtex io bot.

mairatma · 2023-08-03T11:41:53Z

react/package.json

@@ -1,6 +1,6 @@
 {
  "name": "vtex.b2b-quotes",
-  "version": "1.5.5",
+  "version": "1.5.6",


Same here, please revert this line.

mairatma · 2023-08-03T11:42:35Z

react/utils/metrics/createQuote.ts

+      buy_org_id: metricsData.organization,
+      buy_org_name: metricsData.organizationName,


I've just noticed that this PR also has the typos:

Suggested change

buy_org_id: metricsData.organization,

buy_org_name: metricsData.organizationName,

buyer_org_id: metricsData.organization,

buyer_org_name: metricsData.organizationName,

Can you check if there are others and fix them? Same for the metrics for using quotes.

sonarcloud · 2023-08-03T12:09:41Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

The version of Java (11.0.17) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

vtex-io-ci-cd · 2023-08-10T18:53:49Z

Your PR has been merged! App is being published. 🚀
Version 1.5.5 → 1.6.0

After the publishing process has been completed (check #vtex-io-releases) and doing A/B tests with the new version, you can deploy your release by running:

vtex deploy [email protected]

After that your app will be updated on all accounts.

For more information on the deployment process check the docs. 📖

Mauro Takeda added 6 commits July 5, 2023 21:48

Unsafe/incomplete implementation. Just saving working in progress

2e49f30

Partial implementation

29b5577

Include analytics metric for creation quote

d1ab98e

Rename fields style (from camelcase to snake case) because analytics …

fc479f8

…queries extraction demand it

Limit type visibility

3d8e132

Removing unused dependencies

4631ec3

mtakeda requested a review from a team July 14, 2023 13:53

Mauro Takeda added 2 commits July 14, 2023 11:47

Update readme.me with info about create quotation event

17de4e2

Add changelog data

c4667b2

mairatma reviewed Jul 14, 2023

View reviewed changes

docs/README.md Outdated Show resolved Hide resolved

Removing internal data from external documentation

8b39751

mairatma reviewed Jul 14, 2023

View reviewed changes

Mauro Takeda added 3 commits July 17, 2023 09:08

Reverting Readme

299b5e7

Reduzing graphql calls

0d2b784

Await axios call

fc5dc22

mairatma reviewed Jul 19, 2023

View reviewed changes

Mauro Takeda added 5 commits July 19, 2023 11:02

Get account from runtime instead of session and heck user null fields

2f5558d

Fix accountName parameter

b5201e2

Handle graphql errors

9928cf8

Handle null string at costcenter

f1b4c40

Add Use Quote Metrics

6a64cf5

mtakeda requested a review from a team July 24, 2023 11:52

Add change to changelog

dd75260

mairatma previously approved these changes Jul 24, 2023

View reviewed changes

Mauro Takeda added 2 commits July 26, 2023 10:58

Handle async in methods

4f5771e

Merge pull request #56 from vtex-apps/B2BTEAM-1558

6e8e773

Use Quote Metric

mtakeda dismissed mairatma’s stale review via 6e8e773 August 3, 2023 02:59

mtakeda and others added 2 commits August 3, 2023 02:59

Release v1.5.6

65b826c

Changelog

bd5734c

mairatma reviewed Aug 3, 2023

View reviewed changes

Mauro Takeda added 3 commits August 3, 2023 09:02

Revert bump version in manifest

5392102

Revert bump version in package

bf68d1c

Fix typo

4bff5c2

mairatma approved these changes Aug 3, 2023

View reviewed changes

mtakeda merged commit 6112479 into master Aug 10, 2023
10 checks passed

mtakeda deleted the B2BTEAM-1256 branch August 10, 2023 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creation Quote Metric #55

Creation Quote Metric #55

mtakeda commented Jul 14, 2023

vtex-io-ci-cd bot commented Jul 14, 2023 •

edited by mtakeda

Loading

vtex-io-docs-bot bot commented Jul 14, 2023 •

edited by mtakeda

Loading

github-actions bot commented Jul 14, 2023 •

edited

Loading

mairatma Jul 14, 2023

mairatma Jul 14, 2023

mairatma Jul 14, 2023

mairatma Jul 14, 2023

mairatma Jul 14, 2023

mtakeda Jul 17, 2023

mairatma Jul 18, 2023 •

edited

Loading

mairatma Jul 14, 2023

mairatma left a comment

mairatma Jul 19, 2023

mairatma Jul 19, 2023

mtakeda Jul 19, 2023

mairatma Jul 20, 2023

mairatma Jul 19, 2023

mairatma Jul 19, 2023

mairatma left a comment

mairatma commented Jul 24, 2023

mairatma Aug 3, 2023

mairatma Aug 3, 2023

mairatma Aug 3, 2023

sonarcloud bot commented Aug 3, 2023

vtex-io-ci-cd bot commented Aug 10, 2023

	axios.post(ANALYTICS_URL, metric)
	await axios.post(ANALYTICS_URL, metric)

		buy_org_id: metricsData.organization,
		buy_org_name: metricsData.organizationName,

		---

Creation Quote Metric #55

Creation Quote Metric #55

Conversation

mtakeda commented Jul 14, 2023

What problem is this solving?

How to test it?

Screenshots or example usage:

Describe alternatives you've considered, if any.

Related to / Depends on

How does this PR make you feel?

vtex-io-ci-cd bot commented Jul 14, 2023 • edited by mtakeda Loading

vtex-io-docs-bot bot commented Jul 14, 2023 • edited by mtakeda Loading

github-actions bot commented Jul 14, 2023 • 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

mairatma Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mairatma left a 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

mairatma left a comment

Choose a reason for hiding this comment

mairatma commented Jul 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 3, 2023

vtex-io-ci-cd bot commented Aug 10, 2023

vtex-io-ci-cd bot commented Jul 14, 2023 •

edited by mtakeda

Loading

vtex-io-docs-bot bot commented Jul 14, 2023 •

edited by mtakeda

Loading

github-actions bot commented Jul 14, 2023 •

edited

Loading

mairatma Jul 18, 2023 •

edited

Loading