Warn users that they lose their query changes when exiting query builder

[irisyngao]

Summary

Fixes #1274

• Do not alert when exiting query builder opened by right-clicking a class
• only alert users when there are changes in query builder opened from a mapping-test or service

How did you test this change?

• Test(s) added
• Manual testing (please provide screenshots/recordings)
• No testing (please provide an explanation)

Screen.Recording.2022-07-12.at.9.47.36.AM.mov

[changeset-bot]

🦋 Changeset detected

Latest commit: d8fe08e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 23 packages

Name	Type
@finos/legend-query	Patch
@finos/legend-studio-extension-query-builder	Patch
@finos/legend-graph	Patch
@finos/legend-extension-dsl-data-space	Patch
@finos/legend-query-app	Patch
@finos/legend-studio-app	Patch
@finos/legend-application	Patch
@finos/legend-extension-dsl-diagram	Patch
@finos/legend-extension-dsl-persistence	Patch
@finos/legend-extension-dsl-text	Patch
@finos/legend-extension-external-format-json-schema	Patch
@finos/legend-extension-external-language-morphir	Patch
@finos/legend-extension-external-store-service	Patch
@finos/legend-extension-mapping-generation	Patch
@finos/legend-graph-extension-collection	Patch
@finos/legend-manual-tests	Patch
@finos/legend-studio	Patch
@finos/legend-taxonomy	Patch
@finos/legend-query-deployment	Patch
@finos/legend-studio-deployment	Patch
@finos/legend-studio-extension-management-toolkit	Patch
@finos/legend-taxonomy-app	Patch
@finos/legend-taxonomy-deployment	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[MauricioUyaguari]

seems like we have not thought about a couple of things:

Do we want to initialize every query with change detection? Should it be part of the options when you initiate query builder (enableSoftChangeDetection)?

Additionally, I don't think you handle when the user saves the query and then clicks exist. Are you updating the hashLambda when a user has hit save?

[irisyngao]

1. Do we want to initialize every query with change detection? Should it be part of the options when you initiate query builder (enableSoftChangeDetection)?

   For the query builder opened by right-clicking a class, there is no alert anyhow. To achieve this, I default values of QueryBuilderChangeDetectionState.
   

2. Additionally, I don't think you handle when the user saves the query and then clicks exist. Are you updating the hashLambda when a user has hit save?

   hashLambda is not updated when a user hits save but it will be updated every time they reopen the query builder.
   

[MauricioUyaguari]

1. Do we want to initialize every query with change detection? Should it be part of the options when you initiate query builder (enableSoftChangeDetection)?

   For the query builder opened by right-clicking a class, there is no alert anyhow. To achieve this, I default values of QueryBuilderChangeDetectionState.
   

2. Additionally, I don't think you handle when the user saves the query and then clicks exist. Are you updating the hashLambda when a user has hit save?

   hashLambda is not updated when a user hits save but it will be updated every time they reopen the query builder.
   

1. makes sense.
2. I think we need this, couldnt we add this in the save actions the user does? @akphi what do you think

[akphi]

I think we need this, couldnt we add this in the save actions the user does? @akphi what do you think

Yes, this makes sense, and I totally missed this case. Thanks @MauricioUyaguari

[irisyngao]

I think we need this, couldnt we add this in the save actions the user does? @akphi what do you think

Yes, this makes sense, and I totally missed this case. Thanks @MauricioUyaguari

I don't think it bothers us because queryBuilder will be reset(queryBuilderExtension.reset()) every time we click the edit icon. It means even though I save the latest lambda, it will be reset to initial value first then be updated based on latest query when we reopen the query builder.

[MauricioUyaguari]

I think we need this, couldnt we add this in the save actions the user does? @akphi what do you think

Yes, this makes sense, and I totally missed this case. Thanks @MauricioUyaguari

I don't think it bothers us because queryBuilder will be reset(queryBuilderExtension.reset()) every time we click the edit icon. It means even though I save the latest lambda, it will be reset to initial value first then be updated based on latest query when we reopen the query builder.

Yes, the case I'm talking about is when you hit save and the user hits exit, won't we still show this warning? I would expect that during saving we update the lambdaHash

[akphi]

@YannanGao-gs you are relying on the implementation of the save action here, and now since this is a state in QueryBuilderState, it should be considered in a more generic fashion. For example, we need to think how this can work with standalone query builder. But I think for now, this will do, we can leave this be the way you have it

[akphi]

Yes, the case I'm talking about is when you hit save and the user hits exit, won't we still show this warning? I would expect that during saving we update the lambdaHash

Hm, I think right now, when we hit Save Query or so, we also close the embedded query builder modal

[irisyngao]

I think we need this, couldnt we add this in the save actions the user does? @akphi what do you think

Yes, this makes sense, and I totally missed this case. Thanks @MauricioUyaguari

I don't think it bothers us because queryBuilder will be reset(queryBuilderExtension.reset()) every time we click the edit icon. It means even though I save the latest lambda, it will be reset to initial value first then be updated based on latest query when we reopen the query builder.

Yes, the case I'm talking about is when you hit save and the user hits exit, won't we still show this warning? I would expect that during saving we update the lambdaHash

The behaviors are

• Do not alert any when exiting query builder opened by right-clicking a class
• No alerts when clicking save(no difference from the original behavior)
• only alert users when there are changes in query builder opened from a mapping-test or service and they hit exit button

[codecov]

Codecov Report

Merging #1278 (d8fe08e) into master (9c55014) will increase coverage by 0.14%.
The diff coverage is 37.03%.

@@            Coverage Diff             @@
##           master    #1278      +/-   ##
==========================================
+ Coverage   41.56%   41.70%   +0.14%     
==========================================
  Files        1157     1181      +24     
  Lines       52533    52507      -26     
  Branches    11971    11890      -81     
==========================================
+ Hits        21836    21900      +64     
+ Misses      30630    30539      -91     
- Partials       67       68       +1     

Impacted Files	Coverage Δ
...graph/src/graphManager/AbstractPureGraphManager.ts	40.00% <ø> (-20.00%)	⬇️
...rc/models/protocols/pure/v1/V1_PureGraphManager.ts	53.58% <0.00%> (-0.20%)	⬇️
...h/src/models/protocols/pure/v1/engine/V1_Engine.ts	14.35% <0.00%> (ø)
...es/legend-query/src/stores/QueryTextEditorState.ts	25.00% <ø> (ø)
...ry-builder/src/components/EmbeddedQueryBuilder.tsx	67.85% <28.57%> (-13.10%)	⬇️
...kages/legend-query/src/stores/QueryBuilderState.ts	61.03% <33.33%> (-0.90%)	⬇️
...er/src/components/MappingExecutionQueryBuilder.tsx	68.57% <66.66%> (-0.18%)	⬇️
...builder/src/components/MappingTestQueryBuilder.tsx	70.96% <66.66%> (-0.47%)	⬇️
...ery-builder/src/components/ServiceQueryBuilder.tsx	72.50% <66.66%> (-0.48%)	⬇️
...engine/service/V1_DEPRECATED__ServiceTestResult.ts	0.00% <0.00%> (-100.00%)	⬇️
... and 249 more