Popup asking to save before closing for ContingencyStoreEditor #94

miovd · 2018-10-25T15:23:18Z

Note: TODO - same thing in scripts

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 2

Including the following issue(s) which could not be reported in line:

Code Smell: Remove the "gsePane" field and declare it as a local variable in the relevant methods. (more)

See all issues in SonarCloud

mathbagu · 2018-10-26T17:14:18Z

gse-afs-ext-base/src/main/java/com/powsybl/gse/afs/ext/base/ContingencyStoreEditor.java

+                save();
+                return true;
+            } else if (type == dontSaveAndContinue) {
+                saved.set(true);


Seems weird to set saved to true here: this is not logic. Is it really needed?

mathbagu · 2018-10-26T17:15:31Z

gse-afs-ext-base/src/main/resources/lang/ContingencyStore.properties

+UnsavedQuitConfirmation=is not saved, its modifications will be lost. Would you like to save the file first ?
+SaveAndContinue=Save and Continue
+DontSaveAndContinue=Don't save and Continue
+Cancel=Cancel


Sort entries by alphabetical order: it will be easier to find the missing properties

mathbagu · 2018-10-26T17:16:06Z

gse-afs-ext-base/src/main/resources/lang/ContingencyStore_fr.properties

+UnsavedQuitConfirmation=n'est pas sauvegard�, ses modifications seront perdues. Voulez vous enregistrer ce fichier d'abord ?
+SaveAndContinue=Enregistrer et Continuer
+DontSaveAndContinue=Continuer sans enregistrer
+Cancel=Annuler


Same remark

mathbagu · 2018-10-26T17:16:41Z

gse-afs-ext-base/src/main/resources/lang/ContingencyStore_fr.properties

@@ -5,3 +5,7 @@ NewName=Nouveau nom
 Rename=Renommer
 RenameContingency=Renommer la contingence
 Remove=Supprimer
+UnsavedQuitConfirmation=n'est pas sauvegard�, ses modifications seront perdues. Voulez vous enregistrer ce fichier d'abord ?
+SaveAndContinue=Enregistrer et Continuer


Typo issue: Enregistrer et continuer

mathbagu · 2018-10-26T17:17:25Z

gse-app/src/main/java/com/powsybl/gse/app/GsePane.java

        }
    }
+
+    public boolean canBeClosed() {


Rename to isClosable?

mathbagu · 2018-10-26T17:18:37Z

gse-app/src/main/java/com/powsybl/gse/app/ProjectPane.java

@@ -864,4 +874,8 @@ public void dispose() {
        taskItems.dispose();
        closeViews();
    }
+
+    public boolean canBeClosed() {


Same remark

ProjectPane extends Tab which already has a final isClosable() method. I did not rename it here.

mathbagu · 2018-10-26T17:18:45Z

gse-network-explorer/src/main/java/com/powsybl/gse/explorer/NetworkExplorer.java

@@ -360,4 +360,9 @@ public void dispose() {
        projectCase.removeListener(this);
    }

+    @Override
+    public boolean canBeClosed() {


Same remark

mathbagu · 2018-10-26T17:18:52Z

gse-network-map/src/main/java/com/powsybl/gse/map/NetworkMap.java

@@ -196,4 +196,9 @@ public void view() {
    public void dispose() {
        // nothing to dispose
    }
+
+    @Override
+    public boolean canBeClosed() {


Same remark

mathbagu · 2018-10-26T17:18:58Z

gse-security-analysis/src/main/java/com/powsybl/gse/security/SecurityAnalysisResultViewer.java

@@ -139,4 +139,9 @@ public void dispose() {
        preContResultPane.savePreferences();
        postContResultPane.savePreferences();
    }
+
+    @Override
+    public boolean canBeClosed() {


Same remark

mathbagu · 2018-10-26T17:19:05Z

gse-spi/src/main/java/com/powsybl/gse/spi/ProjectFileViewer.java

@@ -19,4 +19,5 @@

    void dispose();

+    boolean canBeClosed();


Same remark

…n alertSave()

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

mathbagu

There is something weird in the logic of GsePane::dispose and GsePane::isClosable: I'm not sure the closable attribute is needed: a GsePane is closable is all the tabs are closable. The dispose method should call isClosable and dispose all the tabs only if the GsePane is closable.

mathbagu · 2018-11-05T22:13:08Z

gse-afs-ext-base/src/main/java/com/powsybl/gse/afs/ext/base/ContingencyStoreEditor.java

@@ -286,6 +286,26 @@ private ContextMenu createContingencyElementMenu() {
        return new ContextMenu(removeItem);
    }

+    private boolean alertSave() {
+        Alert alert = new Alert(Alert.AlertType.CONFIRMATION);
+        alert.setHeaderText("'" + this.store.getName() + "' " + RESOURCE_BUNDLE.getString("UnsavedQuitConfirmation"));


Use MessageFormat seems better:

MessageFormat.format(RESOURCE_BUNDLE.getString("UnsavedQuitConfirmation"), store.getName());

mathbagu

There is a cosmetic issue: the label of the buttons are too long in french and are cut.

I miss something

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

- rename isClosable() (except in ProjectPane which extends a Tab) - check if it is closable before calling dispose() - when deleting a file, the closing is forced even if not saved

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

sonarcloud

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

sonarcloud · 2018-11-09T17:00:33Z

gse-afs-ext-base/src/main/java/com/powsybl/gse/afs/ext/base/ModificationScriptEditor.java

@@ -135,4 +135,10 @@ public void dispose() {
        storableScript.removeListener(this);
    }

+    @Override
+    public boolean isClosable() {
+        //TODO: save before close


Code Smell: Complete the task associated to this TODO comment. (squid:S1135)

See it in SonarCloud

ralambotianamio and others added 2 commits October 25, 2018 17:17

save before closing cse

1170fab

Merge branch 'master' into save_before_closing_cse

9bcf958

miovd requested a review from mathbagu October 25, 2018 15:33

Merge branch 'master' into save_before_closing_cse

554ed1a

sonarcloud bot reviewed Oct 26, 2018

View reviewed changes

mathbagu suggested changes Oct 26, 2018

View reviewed changes

miovd changed the title ~~Popup asking to save before closing for ContingencyStoreEditor~~ [WIP] Popup asking to save before closing for ContingencyStoreEditor Oct 29, 2018

corrections: rename canBeClosed() + alphabetical properties + logic o…

4b82ae4

…n alertSave()

sonarcloud bot reviewed Oct 29, 2018

View reviewed changes

Merge branch 'master' into save_before_closing_cse

3955e6a

sonarcloud bot reviewed Oct 29, 2018

View reviewed changes

miovd changed the title ~~[WIP] Popup asking to save before closing for ContingencyStoreEditor~~ Popup asking to save before closing for ContingencyStoreEditor Oct 29, 2018

Merge branch 'master' into save_before_closing_cse

7e5388c

sonarcloud bot reviewed Nov 5, 2018

View reviewed changes

mathbagu reviewed Nov 5, 2018

View reviewed changes

mathbagu previously requested changes Nov 5, 2018

View reviewed changes

Small fixes

bc1df12

mathbagu changed the title ~~Popup asking to save before closing for ContingencyStoreEditor~~ [WIP] Popup asking to save before closing for ContingencyStoreEditor Nov 5, 2018

sonarcloud bot reviewed Nov 5, 2018

View reviewed changes

ralambotianamio added 3 commits November 7, 2018 15:28

cosmetics

6528d42

correction:

58e6687

- rename isClosable() (except in ProjectPane which extends a Tab) - check if it is closable before calling dispose() - when deleting a file, the closing is forced even if not saved

resolve merge conflict

8d2fc83

sonarcloud bot reviewed Nov 7, 2018

View reviewed changes

miovd changed the title ~~[WIP] Popup asking to save before closing for ContingencyStoreEditor~~ Popup asking to save before closing for ContingencyStoreEditor Nov 7, 2018

Mathieu BAGUE and others added 2 commits November 9, 2018 17:47

Move alert in GseAlerts to mutualize code

e73f008

Merge branch 'master' into save_before_closing_cse

9fc09db

sonarcloud bot reviewed Nov 9, 2018

View reviewed changes

mathbagu merged commit ef79ff7 into master Nov 9, 2018

mathbagu deleted the save_before_closing_cse branch November 9, 2018 21:44

miovd mentioned this pull request Nov 13, 2018

Close project or GSE should warn for unsaved modifications #117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Popup asking to save before closing for ContingencyStoreEditor #94

Popup asking to save before closing for ContingencyStoreEditor #94

miovd commented Oct 25, 2018

sonarcloud bot left a comment

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

miovd Nov 7, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

mathbagu Oct 26, 2018

sonarcloud bot left a comment

sonarcloud bot left a comment

sonarcloud bot left a comment

mathbagu left a comment

mathbagu Nov 5, 2018

mathbagu left a comment •

edited

Loading

sonarcloud bot left a comment

sonarcloud bot left a comment

sonarcloud bot left a comment

sonarcloud bot left a comment

sonarcloud bot Nov 9, 2018

Popup asking to save before closing for ContingencyStoreEditor #94

Popup asking to save before closing for ContingencyStoreEditor #94

Conversation

miovd commented Oct 25, 2018

sonarcloud bot 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

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

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

mathbagu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathbagu left a comment • edited Loading

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot left a comment

Choose a reason for hiding this comment

sonarcloud bot Nov 9, 2018

Choose a reason for hiding this comment

mathbagu left a comment •

edited

Loading