Sync from gdc-ui/sdk hash:b42305b#7345
Merged
Merged
Conversation
added 2 commits
July 8, 2026 03:15
yenkins-admin
approved these changes
Jul 8, 2026
yenkins-admin
left a comment
yenkins-admin
left a comment
Contributor
There was a problem hiding this comment.
Auto-approved by sync workflow
📝 WalkthroughWalkthroughThis PR adds an as-code metric editor to the Analytics Catalog (feature-flagged), including YAML schema/validation/serialization, a metric mutation adapter, create/edit/delete dialogs, and UI wiring. It also adds secret-redaction hooks to goodmock recording utilities, and bumps package versions across the monorepo. ChangesAnalytics Catalog Metric As-Code Editor
Estimated code review effort: 4 (Complex) | ~60 minutes Goodmock Secret Redaction
Estimated code review effort: 3 (Moderate) | ~25 minutes Version Bump and NOTICE Update
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CreateObjectButton
participant MetricCreateDialog
participant MetricDialog
participant MetricMutationPort
participant AnalyticalBackend
User->>CreateObjectButton: Select "Metric"
CreateObjectButton->>MetricCreateDialog: open dialog
MetricCreateDialog->>MetricDialog: render with initialMetric YAML
User->>MetricDialog: edit YAML, submit
MetricDialog->>MetricCreateDialog: validated definition
MetricCreateDialog->>MetricMutationPort: create(definition)
MetricMutationPort->>AnalyticalBackend: createMeasure()
AnalyticalBackend-->>MetricMutationPort: saved measure
MetricMutationPort-->>MetricCreateDialog: created item
MetricCreateDialog->>CreateObjectButton: onCreated / refetch
sequenceDiagram
participant Test
participant snapshotAndSaveRecording
participant Goodmock
participant Consumer
Test->>snapshotAndSaveRecording: options (secretMappings, leakPatterns, sanitizeMappings)
snapshotAndSaveRecording->>Goodmock: take snapshot (repeatsAsScenarios)
Goodmock-->>snapshotAndSaveRecording: mappings
snapshotAndSaveRecording->>snapshotAndSaveRecording: redact resolveSettings tokens
snapshotAndSaveRecording->>Consumer: sanitizeMappings(mappings)
Consumer-->>snapshotAndSaveRecording: sanitized mappings
snapshotAndSaveRecording->>snapshotAndSaveRecording: mask secrets & check leakPatterns
snapshotAndSaveRecording-->>Test: write mapping file
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
libs/sdk-e2e-utils/src/goodmock.ts (1)
362-384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated escaped-secret/masked-pattern computation across the masking and leak-guard loops.
escapedSecretandmaskedFormare independently rebuilt in both the masking loop (365-373) and the leak-guard loop (375-384). Precomputing once per secret and reusing would avoid the two implementations silently diverging if one is edited without the other.♻️ Suggested refactor
- for (const { secret, placeholder } of effectiveSecretMappings) { - const escapedSecret = JSON.stringify(secret).slice(1, -1); - const escapedPlaceholder = JSON.stringify(placeholder).slice(1, -1); - output = output.replaceAll(escapedSecret, escapedPlaceholder); - const maskedForm = buildMaskedFormPattern(escapedSecret); - if (maskedForm) { - output = output.replace(maskedForm, escapedPlaceholder); - } - } - // Leak guard: never write a recording that still contains a real secret (full or masked). - for (const { secret } of effectiveSecretMappings) { - const escapedSecret = JSON.stringify(secret).slice(1, -1); - const maskedForm = buildMaskedFormPattern(escapedSecret); - if (output.includes(escapedSecret) || output.includes(secret) || maskedForm?.test(output)) { + const secretMasks = effectiveSecretMappings.map(({ secret, placeholder }) => { + const escapedSecret = JSON.stringify(secret).slice(1, -1); + const escapedPlaceholder = JSON.stringify(placeholder).slice(1, -1); + return { secret, escapedSecret, escapedPlaceholder, maskedForm: buildMaskedFormPattern(escapedSecret) }; + }); + for (const { escapedSecret, escapedPlaceholder, maskedForm } of secretMasks) { + output = output.replaceAll(escapedSecret, escapedPlaceholder); + if (maskedForm) { + output = output.replace(maskedForm, escapedPlaceholder); + } + } + // Leak guard: never write a recording that still contains a real secret (full or masked). + for (const { secret, escapedSecret, maskedForm } of secretMasks) { + if (output.includes(escapedSecret) || output.includes(secret) || maskedForm?.test(output)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-e2e-utils/src/goodmock.ts` around lines 362 - 384, The masking logic in goodmock’s secret sanitization re-computes escapedSecret and maskedForm separately in the replacement loop and the leak-guard loop, which risks the two paths drifting apart. Refactor the handling around effectiveSecretMappings in goodmock so each secret’s escaped secret and masked pattern are computed once and reused in both replacement and validation, keeping the sanitization and leak-check behavior consistent.libs/sdk-ui-catalog/src/metric/MetricYamlEditor.tsx (1)
1-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate of
ParameterYamlEditor.tsx.This component's structure (state,
useMemoextensions,handleChange, render) is essentially identical toParameterYamlEditor.tsx, differing only in the linter/completions imports and CSS class reuse. Consider extracting a shared genericAsCodeYamlEditorthat acceptslinter/onCompletionas props to avoid two editors drifting out of sync as more as-code editors are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/metric/MetricYamlEditor.tsx` around lines 1 - 48, The MetricYamlEditor component is a near-duplicate of ParameterYamlEditor and should be refactored to avoid drift as more as-code editors are added. Extract the shared state, useMemo extensions, handleChange, and SyntaxHighlightingInput wiring into a generic AsCodeYamlEditor component, and pass the editor-specific pieces (such as createMetricYamlLinter and metricSchemaCompletions) as props or config from MetricYamlEditor. Keep MetricYamlEditor as a thin wrapper that supplies its linter/completions and class name while reusing the shared implementation.libs/sdk-ui-catalog/src/metric/MetricDialog.tsx (1)
1-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftLarge structural duplicate of
ParameterDialog.tsx.
MetricDialogmirrorsParameterDialogalmost line-for-line (state shape,validate/handleSubmit/handleDuplicate/footerLeftRenderer,ConfirmDialogwiring), differing mainly in message ids, docs URL, and the validate/serialize functions it delegates to. This is significant duplicated logic across two feature areas that will need to be kept in sync for every future bugfix (e.g., the submit-disabled logic, error-message mapping, duplicate flow). Consider extracting a shared genericAsCodeDialogparameterized by the validate/serialize functions, docs URL, and message bundle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/metric/MetricDialog.tsx` around lines 1 - 227, `MetricDialog` is a near-copy of `ParameterDialog`, with the same state, submit/duplicate handling, footer rendering, and `ConfirmDialog` wiring repeated here. Refactor the shared dialog behavior into a reusable `AsCodeDialog` (or equivalent) and parameterize it with the metric-specific pieces: validation/serialization functions, docs URL, and message bundle. Keep `MetricDialog` as a thin wrapper that supplies metric-specific dependencies and symbols like `handleSubmit`, `handleDuplicate`, `footerLeftRenderer`, and `MetricYamlEditor`.libs/sdk-ui-catalog/src/metric/MetricDetailActions.tsx (1)
47-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
onOpentype doesn't match the actual event union it can receive.
handleActionsMenuSelectacceptsMouseEvent | KeyboardEvent(keyboard-triggered menu selection), butonOpenis typed to only acceptMouseEvent, forcing a cast at the call site that misrepresents the runtime type for keyboard-triggered opens.♻️ Proposed fix
- onOpen?: (event: MouseEvent, openEvent: OpenHandlerEvent) => void; + onOpen?: (event: MouseEvent | KeyboardEvent, openEvent: OpenHandlerEvent) => void;if (actionId === "open") { - onOpen?.(event as MouseEvent, { + onOpen?.(event, { item, workspaceId, newTab: event.metaKey || event.ctrlKey, preventDefault: event.preventDefault.bind(event), });Also applies to: 114-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/metric/MetricDetailActions.tsx` around lines 47 - 56, Update the MetricDetailActions component API so onOpen in IMetricDetailActionsProps matches the actual event shape handled by handleActionsMenuSelect, which can pass either a mouse or keyboard event. Adjust the onOpen callback typing and any related call sites in MetricDetailActions to use the shared union type instead of casting, so keyboard-triggered opens are represented correctly without unsafe assumptions.libs/sdk-ui-catalog/src/metric/MetricDeleteDialog.tsx (1)
61-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider surfacing the backend-provided delete failure reason.
extractBackendErrorDetail(added inutils/backendError.ts) is used byParameterDialog.tsxto surface curated backend reasons (e.g., dependency conflicts, limits). Here, any delete failure — including a specific "still referenced" error from the backend — collapses to the same genericdeleteErrortoast, losing actionable detail for the user.♻️ Illustrative fix (adapt to `useToastMessage` API)
+import { extractBackendErrorDetail } from "../utils/backendError.js"; + const handleDelete = useCallback(async () => { setIsDeleting(true); try { await mutation.delete(item); onDeleted(); onClose(); addSuccess(messages.deleteSuccess); - } catch { - addError(messages.deleteError); + } catch (error) { + addError(extractBackendErrorDetail(error) ?? messages.deleteError); setIsDeleting(false); } }, [addError, addSuccess, item, mutation, onClose, onDeleted]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/metric/MetricDeleteDialog.tsx` around lines 61 - 72, The delete flow in MetricDeleteDialog’s handleDelete swallows the backend error, so all failures show the same generic toast. Update the catch block to capture the thrown error from mutation.delete(item) and pass it through extractBackendErrorDetail (as used in ParameterDialog and utils/backendError.ts) before calling addError, so backend-specific reasons like “still referenced” are surfaced to the user. Keep the existing success path unchanged and preserve the isDeleting reset on failure.libs/sdk-ui-catalog/src/tests/AnalyticsCatalogDetail.test.tsx (1)
86-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd negative-path coverage for the metric editor feature flag.
Only the enabled-flag path is tested. Consider adding a case with
enableAnalyticalCatalogMetricEditor: false(orcanEdit: false) asserting the metric edit UI does not render, to lock in the gating behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/tests/AnalyticsCatalogDetail.test.tsx` around lines 86 - 99, Add a negative-path test for the metric editor gating in AnalyticsCatalogDetail.test.tsx: the current test only covers the enabled path in AnalyticsCatalogDetailContent. Add a case using the same setup but with enableAnalyticalCatalogMetricEditor disabled (or canEdit set to false) and assert the edit button/action bar does not render. Keep the coverage alongside the existing render test so the flag behavior is locked in.libs/sdk-ui-catalog/src/header/CreateObjectButton.tsx (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLogic looks correct; verify test coverage for the new metric-create branch.
The conditional redirect/in-catalog split and
handleSelectrouting forObjectTypes.METRIClook correct. The providedCreateObjectButton.test.tsxdiff only touches the parameter-creation error path; no test appears to exerciseshowMetricEditor=trueopeningMetricCreateDialogor verifyhandleMetricCreated/refetchObjectType(ObjectTypes.METRIC).Also applies to: 40-97, 108-115, 160-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk-ui-catalog/src/header/CreateObjectButton.tsx` at line 19, The new metric-create branch in CreateObjectButton needs test coverage. Add or update tests in CreateObjectButton.test.tsx to exercise the showMetricEditor=true path that renders MetricCreateDialog, and verify the metric flow calls handleMetricCreated and refetchObjectType(ObjectTypes.METRIC) after creation. Use the CreateObjectButton, handleSelect, and handleMetricCreated paths to locate the relevant behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/sdk-e2e-utils/src/goodmock.ts`:
- Around line 310-314: Add the missing no-console suppression before the new
console.warn in goodmock.ts so it matches the existing console.* usages in this
file; update the catch block around sanitizeCredentials / resolveSettings to
include the same // eslint-disable-next-line no-console comment pattern used
elsewhere, keeping the warning call unchanged.
- Around line 233-274: The snapshot flow in snapshotAndSaveRecording parses the
response body without checking whether fetch() returned an error status. Add an
explicit snapshotRes.ok guard immediately after the POST to
/__admin/recordings/snapshot, and surface a clear snapshot-specific error before
calling snapshotRes.json(). Keep the fix localized to snapshotAndSaveRecording
so non-2xx goodmock failures don’t turn into misleading JSON parse exceptions.
In `@libs/sdk-ui-catalog/styles/scss/detail.scss`:
- Line 6: The `@use` import in the stylesheet is using the explicit .scss
extension, which triggers the scss/load-partial-extension lint rule. Update the
import in detail.scss to reference the partial by its basename only, and keep
the change localized to the existing `@use` statement for codeDialog.scss.
---
Nitpick comments:
In `@libs/sdk-e2e-utils/src/goodmock.ts`:
- Around line 362-384: The masking logic in goodmock’s secret sanitization
re-computes escapedSecret and maskedForm separately in the replacement loop and
the leak-guard loop, which risks the two paths drifting apart. Refactor the
handling around effectiveSecretMappings in goodmock so each secret’s escaped
secret and masked pattern are computed once and reused in both replacement and
validation, keeping the sanitization and leak-check behavior consistent.
In `@libs/sdk-ui-catalog/src/header/CreateObjectButton.tsx`:
- Line 19: The new metric-create branch in CreateObjectButton needs test
coverage. Add or update tests in CreateObjectButton.test.tsx to exercise the
showMetricEditor=true path that renders MetricCreateDialog, and verify the
metric flow calls handleMetricCreated and refetchObjectType(ObjectTypes.METRIC)
after creation. Use the CreateObjectButton, handleSelect, and
handleMetricCreated paths to locate the relevant behavior.
In `@libs/sdk-ui-catalog/src/metric/MetricDeleteDialog.tsx`:
- Around line 61-72: The delete flow in MetricDeleteDialog’s handleDelete
swallows the backend error, so all failures show the same generic toast. Update
the catch block to capture the thrown error from mutation.delete(item) and pass
it through extractBackendErrorDetail (as used in ParameterDialog and
utils/backendError.ts) before calling addError, so backend-specific reasons like
“still referenced” are surfaced to the user. Keep the existing success path
unchanged and preserve the isDeleting reset on failure.
In `@libs/sdk-ui-catalog/src/metric/MetricDetailActions.tsx`:
- Around line 47-56: Update the MetricDetailActions component API so onOpen in
IMetricDetailActionsProps matches the actual event shape handled by
handleActionsMenuSelect, which can pass either a mouse or keyboard event. Adjust
the onOpen callback typing and any related call sites in MetricDetailActions to
use the shared union type instead of casting, so keyboard-triggered opens are
represented correctly without unsafe assumptions.
In `@libs/sdk-ui-catalog/src/metric/MetricDialog.tsx`:
- Around line 1-227: `MetricDialog` is a near-copy of `ParameterDialog`, with
the same state, submit/duplicate handling, footer rendering, and `ConfirmDialog`
wiring repeated here. Refactor the shared dialog behavior into a reusable
`AsCodeDialog` (or equivalent) and parameterize it with the metric-specific
pieces: validation/serialization functions, docs URL, and message bundle. Keep
`MetricDialog` as a thin wrapper that supplies metric-specific dependencies and
symbols like `handleSubmit`, `handleDuplicate`, `footerLeftRenderer`, and
`MetricYamlEditor`.
In `@libs/sdk-ui-catalog/src/metric/MetricYamlEditor.tsx`:
- Around line 1-48: The MetricYamlEditor component is a near-duplicate of
ParameterYamlEditor and should be refactored to avoid drift as more as-code
editors are added. Extract the shared state, useMemo extensions, handleChange,
and SyntaxHighlightingInput wiring into a generic AsCodeYamlEditor component,
and pass the editor-specific pieces (such as createMetricYamlLinter and
metricSchemaCompletions) as props or config from MetricYamlEditor. Keep
MetricYamlEditor as a thin wrapper that supplies its linter/completions and
class name while reusing the shared implementation.
In `@libs/sdk-ui-catalog/src/tests/AnalyticsCatalogDetail.test.tsx`:
- Around line 86-99: Add a negative-path test for the metric editor gating in
AnalyticsCatalogDetail.test.tsx: the current test only covers the enabled path
in AnalyticsCatalogDetailContent. Add a case using the same setup but with
enableAnalyticalCatalogMetricEditor disabled (or canEdit set to false) and
assert the edit button/action bar does not render. Keep the coverage alongside
the existing render test so the flag behavior is locked in.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8eb5725c-5b95-48f7-90b9-2b1266801605
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (126)
NOTICEcommon/changes/@gooddata/sdk-ui-all/c.jma-F1-2657-as-code-metric-editor_2026-07-01-12-04-41.jsoncommon/changes/@gooddata/sdk-ui-all/gen-ai-llm-test-secret-redaction_2026-07-06-05-10-18.jsoncommon/config/rush/version-policies.jsonexamples/playground/package.jsonexamples/sdk-interactive-examples/examples-template/package.jsonexamples/sdk-interactive-examples/examples/example-attributefilter/package.jsonexamples/sdk-interactive-examples/examples/example-chartconfig/package.jsonexamples/sdk-interactive-examples/examples/example-columnchart/package.jsonexamples/sdk-interactive-examples/examples/example-combochart/package.jsonexamples/sdk-interactive-examples/examples/example-dashboard/package.jsonexamples/sdk-interactive-examples/examples/example-datefilter/package.jsonexamples/sdk-interactive-examples/examples/example-dependentfilters/package.jsonexamples/sdk-interactive-examples/examples/example-execute/package.jsonexamples/sdk-interactive-examples/examples/example-granularity/package.jsonexamples/sdk-interactive-examples/examples/example-headline/package.jsonexamples/sdk-interactive-examples/examples/example-pivottable/package.jsonexamples/sdk-interactive-examples/examples/example-relativedatefilter/package.jsonexamples/sdk-interactive-examples/examples/example-repeater/package.jsonexamples/sdk-interactive-examples/package.jsonlibs/api-client-tiger/package.jsonlibs/sdk-backend-base/package.jsonlibs/sdk-backend-mockingbird/package.jsonlibs/sdk-backend-spi/package.jsonlibs/sdk-backend-tiger/package.jsonlibs/sdk-backend-tiger/src/backend/features/feature.tslibs/sdk-backend-tiger/src/backend/uiFeatures.tslibs/sdk-code-convertors/package.jsonlibs/sdk-code-convertors/python/pyproject.tomllibs/sdk-code-schemas/package.jsonlibs/sdk-e2e-utils/api/sdk-e2e-utils.api.mdlibs/sdk-e2e-utils/package.jsonlibs/sdk-e2e-utils/src/goodmock.tslibs/sdk-e2e-utils/src/index.tslibs/sdk-e2e-utils/src/playwright.tslibs/sdk-embedding/package.jsonlibs/sdk-model/api/sdk-model.api.mdlibs/sdk-model/package.jsonlibs/sdk-model/src/settings/settings.tslibs/sdk-pluggable-application-model/package.jsonlibs/sdk-ui-all/package.jsonlibs/sdk-ui-application-header/package.jsonlibs/sdk-ui-catalog/package.jsonlibs/sdk-ui-catalog/src/AnalyticsCatalogDetail.tsxlibs/sdk-ui-catalog/src/catalog/Catalog.tsxlibs/sdk-ui-catalog/src/catalogDetail/CatalogDetailActionBar.tsxlibs/sdk-ui-catalog/src/catalogDetail/CatalogDetailActions.tsxlibs/sdk-ui-catalog/src/catalogDetail/CatalogDetailContent.tsxlibs/sdk-ui-catalog/src/catalogItem/query.tslibs/sdk-ui-catalog/src/header/CreateObjectButton.tsxlibs/sdk-ui-catalog/src/header/tests/CreateObjectButton.test.tsxlibs/sdk-ui-catalog/src/localization/bundles/en-US.jsonlibs/sdk-ui-catalog/src/main/Main.tsxlibs/sdk-ui-catalog/src/metric/MetricCreateDialog.tsxlibs/sdk-ui-catalog/src/metric/MetricDeleteDialog.tsxlibs/sdk-ui-catalog/src/metric/MetricDetailActions.tsxlibs/sdk-ui-catalog/src/metric/MetricDialog.tsxlibs/sdk-ui-catalog/src/metric/MetricEditDialog.tsxlibs/sdk-ui-catalog/src/metric/MetricMutationContext.tsxlibs/sdk-ui-catalog/src/metric/MetricYamlEditor.tsxlibs/sdk-ui-catalog/src/metric/gate.tslibs/sdk-ui-catalog/src/metric/metricCompletions.tslibs/sdk-ui-catalog/src/metric/metricConverter.tslibs/sdk-ui-catalog/src/metric/metricCopy.tslibs/sdk-ui-catalog/src/metric/metricMutationPort.tslibs/sdk-ui-catalog/src/metric/metricSchema.tslibs/sdk-ui-catalog/src/metric/metricSerialization.tslibs/sdk-ui-catalog/src/metric/metricValidation.tslibs/sdk-ui-catalog/src/metric/metricYamlLinter.tslibs/sdk-ui-catalog/src/metric/tests/MetricDeleteDialog.test.tsxlibs/sdk-ui-catalog/src/metric/tests/MetricDetailActions.test.tsxlibs/sdk-ui-catalog/src/metric/tests/metricConverter.test.tslibs/sdk-ui-catalog/src/metric/tests/metricCopy.test.tslibs/sdk-ui-catalog/src/metric/tests/metricMutationPort.test.tslibs/sdk-ui-catalog/src/metric/tests/metricMutationPort.test.utils.tslibs/sdk-ui-catalog/src/metric/tests/metricSchema.test.tslibs/sdk-ui-catalog/src/metric/tests/metricSerialization.test.tslibs/sdk-ui-catalog/src/metric/tests/metricValidation.test.tslibs/sdk-ui-catalog/src/parameter/ParameterDialog.tsxlibs/sdk-ui-catalog/src/parameter/ParameterYamlEditor.tsxlibs/sdk-ui-catalog/src/parameter/yamlLinter.tslibs/sdk-ui-catalog/src/tests/AnalyticsCatalogDetail.test.tsxlibs/sdk-ui-catalog/src/utils/backendError.tslibs/sdk-ui-catalog/src/utils/tests/backendError.test.tslibs/sdk-ui-catalog/src/utils/yamlSyntaxLinter.tslibs/sdk-ui-catalog/styles/scss/codeDialog.scsslibs/sdk-ui-catalog/styles/scss/detail.scsslibs/sdk-ui-charts/package.jsonlibs/sdk-ui-dashboard/package.jsonlibs/sdk-ui-ext/package.jsonlibs/sdk-ui-filters/package.jsonlibs/sdk-ui-gen-ai/package.jsonlibs/sdk-ui-geo/package.jsonlibs/sdk-ui-kit/package.jsonlibs/sdk-ui-loaders/package.jsonlibs/sdk-ui-pivot/package.jsonlibs/sdk-ui-pluggable-application/package.jsonlibs/sdk-ui-pluggable-host/package.jsonlibs/sdk-ui-semantic-search/package.jsonlibs/sdk-ui-tests-app/package.jsonlibs/sdk-ui-tests-e2e/package.jsonlibs/sdk-ui-tests-reference-workspace/package.jsonlibs/sdk-ui-tests-scenarios/package.jsonlibs/sdk-ui-tests-storybook/package.jsonlibs/sdk-ui-theme-provider/package.jsonlibs/sdk-ui-vis-commons/package.jsonlibs/sdk-ui-web-components/package.jsonlibs/sdk-ui/package.jsonlibs/util/package.jsontemplates/pluggable-module/harness/package.jsontemplates/pluggable-module/module/package.jsontools/app-toolkit/package.jsontools/applink/package.jsontools/catalog-export/package.jsontools/create-pluggable-module/package.jsontools/dashboard-plugin-template/package.jsontools/eslint-config/package.jsontools/i18n-toolkit/package.jsontools/lint-config/package.jsontools/mock-handling/package.jsontools/oxlint-config/package.jsontools/plugin-toolkit/package.jsontools/react-app-template/package.jsontools/reference-workspace-mgmt/package.jsontools/reference-workspace/package.jsontools/stylelint-config/package.json
Comment on lines
+233
to
+274
| * A single snapshot is taken with repeatsAsScenarios enabled: goodmock turns a | ||
| * request into a scenario chain only when its responses differ across the | ||
| * recording, so stateful sequences are preserved and everything else stays flat. | ||
| * | ||
| * @param host - | ||
| * @param mappingFilePath - | ||
| * @param workspaceIdMappings - Optional source/target workspace ID rewrite(s) | ||
| * applied before mappings are saved. Accepts a single mapping or an array. | ||
| * @param backendHost - Optional real backend URL (e.g. "https://example.gooddata.com"). | ||
| * When provided together with baseUrl, all occurrences are replaced in the | ||
| * saved mappings so that recorded responses (e.g. geo tile URLs) resolve | ||
| * correctly during replay. | ||
| * @param baseUrl - Optional app base URL (e.g. "http://kpi-dashboards-ui:9500") | ||
| * that replaces backendHost references in the saved mappings. | ||
| * @param host - Goodmock host:port (e.g. "backend-mock:8080") | ||
| * @param mappingFilePath - Absolute path to write the mapping JSON file to | ||
| * @param options - See {@link ISnapshotAndSaveRecordingOptions}. | ||
| */ | ||
| export async function snapshotAndSaveRecording( | ||
| host: string, | ||
| mappingFilePath: string, | ||
| workspaceIdMappings?: IWorkspaceIdMapping | IWorkspaceIdMapping[], | ||
| backendHost?: string, | ||
| baseUrl?: string, | ||
| options: ISnapshotAndSaveRecordingOptions = {}, | ||
| ): Promise<void> { | ||
| // Snapshot executionResults with scenarios | ||
| const scenariosRes = await fetch(`http://${host}/__admin/recordings/snapshot`, { | ||
| const { workspaceIdMappings, backendHost, baseUrl, secretMappings, leakPatterns, sanitizeMappings } = | ||
| options; | ||
| // Snapshot everything with scenarios enabled. goodmock only emits a scenario | ||
| // chain when a given request produced *different* responses across the | ||
| // recording; identical repeats collapse to a single mapping. This captures | ||
| // stateful sequences (e.g. a GET whose result changes after a POST) that the | ||
| // old per-URL dedup path silently dropped, while keeping idempotent traffic flat. | ||
| const snapshotRes = await fetch(`http://${host}/__admin/recordings/snapshot`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| repeatsAsScenarios: true, | ||
| filters: { urlPattern: ".*executionResults.*" }, | ||
| persist: false, | ||
| ...snapshotParams, | ||
| }), | ||
| }); | ||
| const scenariosData = (await scenariosRes.json()) as { mappings: IGoodmockMapping[] }; | ||
| const snapshotData = (await snapshotRes.json()) as { mappings: IGoodmockMapping[] }; | ||
|
|
||
| // Snapshot everything else without scenarios | ||
| const plainRes = await fetch(`http://${host}/__admin/recordings/snapshot`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| repeatsAsScenarios: false, | ||
| filters: { urlPattern: "((?!executionResults).)*" }, | ||
| persist: false, | ||
| ...snapshotParams, | ||
| }), | ||
| }); | ||
| const plainData = (await plainRes.json()) as { mappings: IGoodmockMapping[] }; | ||
| const mappings = [...snapshotData.mappings]; | ||
|
|
||
| const mappings = [...plainData.mappings, ...scenariosData.mappings]; | ||
| // An empty snapshot means the recording journal was lost (e.g. a worker crash re-ran the | ||
| // beforeAll reset mid-spec) or the spec made no requests at all. Never overwrite a previous | ||
| // recording with an empty one, and fail the run so the broken recording cannot go unnoticed. | ||
| if (mappings.length === 0) { | ||
| throw new Error( | ||
| `Recording snapshot for ${mappingFilePath} is empty — keeping the existing mapping ` + | ||
| `file. Investigate (worker crash mid-spec? goodmock proxy misconfigured?) and re-record.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `snapshotParams` is declared anywhere in this file.
rg -n 'snapshotParams' libs/sdk-e2e-utils/src/goodmock.tsRepository: gooddata/gooddata-ui-sdk
Length of output: 226
🏁 Script executed:
#!/bin/bash
sed -n '110,140p' libs/sdk-e2e-utils/src/goodmock.ts
printf '\n---\n'
sed -n '233,285p' libs/sdk-e2e-utils/src/goodmock.tsRepository: gooddata/gooddata-ui-sdk
Length of output: 3821
Handle non-2xx snapshot responses before parsing JSON.
fetch() can return an error response here; check snapshotRes.ok first so goodmock failures surface as a clear snapshot error instead of a JSON parse exception.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/sdk-e2e-utils/src/goodmock.ts` around lines 233 - 274, The snapshot flow
in snapshotAndSaveRecording parses the response body without checking whether
fetch() returned an error status. Add an explicit snapshotRes.ok guard
immediately after the POST to /__admin/recordings/snapshot, and surface a clear
snapshot-specific error before calling snapshotRes.json(). Keep the fix
localized to snapshotAndSaveRecording so non-2xx goodmock failures don’t turn
into misleading JSON parse exceptions.
Comment on lines
310
to
314
| } catch { | ||
| console.warn( | ||
| `sanitizeCredentials – resolveSettings body is not valid JSON for url: ${requestUrl}`, | ||
| `sanitizeCredentials – resolveSettings body is not valid JSON for: ${requestPath}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Missing no-console eslint-disable comment on the new console.warn.
Every other console.* call in this file (e.g. lines 222-223, 405-406) is preceded by // eslint-disable-next-line no-console. This new one isn't, which will likely fail lint.
🧹 Suggested fix
} catch {
+ // eslint-disable-next-line no-console
console.warn(
`sanitizeCredentials – resolveSettings body is not valid JSON for: ${requestPath}`,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch { | |
| console.warn( | |
| `sanitizeCredentials – resolveSettings body is not valid JSON for url: ${requestUrl}`, | |
| `sanitizeCredentials – resolveSettings body is not valid JSON for: ${requestPath}`, | |
| ); | |
| } | |
| } catch { | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| `sanitizeCredentials – resolveSettings body is not valid JSON for: ${requestPath}`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/sdk-e2e-utils/src/goodmock.ts` around lines 310 - 314, Add the missing
no-console suppression before the new console.warn in goodmock.ts so it matches
the existing console.* usages in this file; update the catch block around
sanitizeCredentials / resolveSettings to include the same //
eslint-disable-next-line no-console comment pattern used elsewhere, keeping the
warning call unchanged.
|
|
||
| @use "./objectType.scss"; | ||
| @use "./parameter.scss"; | ||
| @use "./codeDialog.scss"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the .scss extension in the @use path.
Stylelint's scss/load-partial-extension flags this import.
🔧 Proposed fix
-@use "./codeDialog.scss";
+@use "./codeDialog";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @use "./codeDialog.scss"; | |
| `@use` "./codeDialog"; |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 6-6: Unexpected extension ".scss" in @use (scss/load-partial-extension)
(scss/load-partial-extension)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/sdk-ui-catalog/styles/scss/detail.scss` at line 6, The `@use` import in
the stylesheet is using the explicit .scss extension, which triggers the
scss/load-partial-extension lint rule. Update the import in detail.scss to
reference the partial by its basename only, and keep the change localized to the
existing `@use` statement for codeDialog.scss.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated sync of gdc-ui → gooddata-ui-sdk Source hash: b42305b
Summary by CodeRabbit
New Features
Bug Fixes
Chores