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

feat: attribute filter with secondary label #5033

Merged
merged 4 commits into from
Jun 20, 2024
Merged

feat: attribute filter with secondary label #5033

merged 4 commits into from
Jun 20, 2024

Conversation

ivan-nejezchleb
Copy link
Contributor

  • secondary label does not affect execution anymore
  • primary label is always used for execution
  • secondary label is stored outside of filter for UI only
  • each AF in Insight has now localIdentifier
  • secondary label references Af via localId
  • displayForms now contain isPrimary, isDefault flags
  • AF keeps its selection when secondary label switched
  • change of secondary label does not recreate AF handler
  • collectLabelElements can now filter by primary label

JIRA: LX-183
risk: high


Important

Please, don't forget to run rush change for the commits that introduce new features 🙏


Refer to documentation to see how to run checks and tests in the pull request. This is the list of the most used commands:

extended test - backstop
extended test - tiger-cypress - integrated
extended test - tiger-cypress - isolated
extended test - tiger-cypress - record

@gooddata
Copy link

gooddata bot commented Jun 19, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20900/ finished with 'FAILURE' status.

@@ -46,13 +46,16 @@ const commonGroupableCatalogItemModifications =
const tigerLabelToDisplayFormMd = (
label: JsonApiLabelOutWithLinks,
attributeRef: ObjRef,
defaultLabelId?: string,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this can be undefined? It's only for backward compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same convertor is used also for geoLabels manipulation and there the default is not provided as it is not relevant


function generateLocalId(prefix: string, objRef: ObjRef, inObject: IAttributeElements): string {
const hasher = new SparkMD5();
hasher.append(JSON.stringify(inObject));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if safe stringify should not be used. Probably not entirely necessary here?
import stringify from "json-stable-stringify";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used same code as for generating other missing localIdentifiers, eg. libs/sdk-model/src/execution/measure/factory.ts

@gooddata
Copy link

gooddata bot commented Jun 19, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20901/ finished with 'FAILURE' status.

@ivan-nejezchleb
Copy link
Contributor Author

ok to test

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20903/ finished with 'FAILURE' status.

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20904/ finished with 'SUCCESS' status.

@ivan-nejezchleb
Copy link
Contributor Author

extended test - backstop

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-pull-request-dispatcher-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-pull-request-dispatcher-pipeline/2200/ finished with 'UNSTABLE' status.

@ivan-nejezchleb
Copy link
Contributor Author

extended test - backstop

@ivan-nejezchleb
Copy link
Contributor Author

extended test - tiger-cypress - isolated

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20906/ finished with 'SUCCESS' status.

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-pull-request-dispatcher-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-pull-request-dispatcher-pipeline/2202/ finished with 'UNSTABLE' status.

@ivan-nejezchleb
Copy link
Contributor Author

extended test - tiger-cypress - isolated

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20907/ finished with 'SUCCESS' status.

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-pull-request-dispatcher-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-pull-request-dispatcher-pipeline/2203/ finished with 'UNSTABLE' status.

- secondary label does not affect execution anymore
- primary label is always used for execution
- secondary label is stored outside of filter for UI only
- each AF in Insight has now localIdentifier
- secondary label references Af via localId
- displayForms now contain isPrimary, isDefault flags
- AF keeps its selection when secondary label switched
- change of secondary label does not recreate AF handler
- collectLabelElements can now filter by primary label

JIRA: LX-183
risk: high
- fix FF propagation
- update test snapshots
JIRA: LX-183
risk: low
JIRA: LX-183
risk: low
@ivan-nejezchleb
Copy link
Contributor Author

extended test - tiger-cypress - isolated

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20910/ finished with 'SUCCESS' status.

@ivan-nejezchleb
Copy link
Contributor Author

extended test - tiger-cypress - integrated

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-pull-request-dispatcher-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-pull-request-dispatcher-pipeline/2205/ finished with 'SUCCESS' status.

@xMort xMort added this pull request to the merge queue Jun 20, 2024
@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-compatibility-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-compatibility-pipeline/801/ finished with 'SUCCESS' status.

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-cypress-tiger-isolated-zuul-pipeline at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-cypress-tiger-isolated-zuul-pipeline/2072/ finished with 'SUCCESS' status.

@gooddata
Copy link

gooddata bot commented Jun 20, 2024

Job gooddata-ui-sdk-unit-tests at https://checklist.intgdc.com/job/gooddata-ui-sdk/job/gooddata-ui-sdk-unit-tests/20911/ finished with 'SUCCESS' status.

Merged via the queue into master with commit c78e1a9 Jun 20, 2024
3 checks passed
@xMort xMort deleted the ine-lx-183 branch June 20, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants