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

ref(insights): pass in organization to getTransactionSummaryBaseUrl #84229

Merged

Conversation

DominikB2014
Copy link
Contributor

@DominikB2014 DominikB2014 commented Jan 29, 2025

As part of #84021 , we have to update the route for the transaction summary to be /insights/summary instead of /performance/summary. This route change should be behind a flag.

We have a helper function that returns the transaction summary url getTransactionSummaryBaseUrl, this function makes the most sense to add the conditional that returns either /insights/summary or /performance/summary. However the helper function does not have access to organization features.

This PR replaces the orgSlug parameter with the organization parameter, that way we can access organization.features. In another PR we can use this parameter to add the conditional on the feature flag.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 29, 2025
Copy link

codecov bot commented Jan 29, 2025

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
8604 2 8602 0
View the top 2 failed tests by shortest run time
spanDetailsRouteWithQuery should encode slashes in span op
Stack Traces | 0.019s run time
TypeError: Cannot read properties of undefined (reading 'slug')
    at getTransactionSummaryBaseUrl (.../performance/transactionSummary/utils.tsx:211:61)
    at generateSpanDetailsRoute (.../transactionSpans/spanDetails/utils.tsx:19:53)
    at spanDetailsRouteWithQuery (.../transactionSpans/spanDetails/utils.tsx:29:20)
    at Object.<anonymous> (.../transactionSpans/spanDetails/index.spec.tsx:715:57)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at runNextTicks (node:internal/process/task_queues:65:5)
    at listOnTimeout (node:internal/timers:555:9)
    at processTimers (node:internal/timers:529:7)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
EventView.getPerformanceTransactionEventsViewUrlTarget() generates a URL with non-customer domain context
Stack Traces | 0.029s run time
Error: expect(received).toBe(expected) // Object.is equality

Expected: ".../performance/summary/events/"
Received: ".../performance/summary/events/"
    at Object.<anonymous> (.../utils/discover/eventView.spec.tsx:3161:29)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@DominikB2014 DominikB2014 marked this pull request as ready for review January 29, 2025 15:56
@DominikB2014 DominikB2014 requested review from a team as code owners January 29, 2025 15:56
@DominikB2014 DominikB2014 merged commit a9689fd into master Jan 29, 2025
44 checks passed
@DominikB2014 DominikB2014 deleted the DominikB2014/pass-in-org-for-transaction-summary-route branch January 29, 2025 18:19
andrewshie-sentry pushed a commit that referenced this pull request Jan 29, 2025
…84229)

As part of #84021 , we have to update the route for the transaction
summary to be `/insights/summary` instead of `/performance/summary`.
This route change should be behind a flag.

We have a helper function that returns the transaction summary url
`getTransactionSummaryBaseUrl`, this function makes the most sense to
add the conditional that returns either /insights/summary or
/performance/summary. However the helper function does not have access
to organization features.

This PR replaces the `orgSlug` parameter with the `organization`
parameter, that way we can access `organization.features`. In another PR
we can use this parameter to add the conditional on the feature flag.
c298lee pushed a commit that referenced this pull request Jan 29, 2025
…84229)

As part of #84021 , we have to update the route for the transaction
summary to be `/insights/summary` instead of `/performance/summary`.
This route change should be behind a flag.

We have a helper function that returns the transaction summary url
`getTransactionSummaryBaseUrl`, this function makes the most sense to
add the conditional that returns either /insights/summary or
/performance/summary. However the helper function does not have access
to organization features.

This PR replaces the `orgSlug` parameter with the `organization`
parameter, that way we can access `organization.features`. In another PR
we can use this parameter to add the conditional on the feature flag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants