PTBAS-741: Adjust Mapping Dialog #186
Conversation
* Added tooltip * Enabled search in comboboxes * Expired token now shown in red * Fixed SyncDialog auto-close * Made auto-deletion of obsolete mappings optional (Keep/Remove)
Death111
changed the base branch from
develop
to
feature/PTBAS-738_syncDialogAnpassungen
# Conflicts: # src/main/java/de/doubleslash/keeptime/controller/HeimatController.java # src/main/java/de/doubleslash/keeptime/view/ExternalProjectsSyncController.java # src/main/java/de/doubleslash/keeptime/viewpopup/SearchCombobox.java # src/test/java/de/doubleslash/keeptime/controller/HeimatControllerTest.java
| if (optionalExistingMapping.isPresent()) { | ||
| final Mapping existingMapping = optionalExistingMapping.get(); | ||
|
|
||
| // Ensure we merge projects robustly: include any projects mapped to the same Heimat task |
There was a problem hiding this comment.
I think I would like to only have projects shown in the dialog, which I also worked on at that date. not all projects which are mapped to it
There was a problem hiding this comment.
hm okay. but when I dont have the time in keeptime but heimat, what should it show for keeptime projects? not so sure if no projects would be wanted (which would be the case when adapting as I wanted it)
| // shouldOnlyUpdateHeimatWhenSomethingHasChanged (not needed - user should decide) | ||
|
|
||
| @Test | ||
| void shouldNotCreateDuplicateHeimatEntryWhenMultipleProjectsMappedAndSomeHaveWork() { |
There was a problem hiding this comment.
I do not think that this "GIVEN" created the issue observed. Running this test on previous code fails yes - but not because the report has 2 rows in the end - which was the issue that we see the same twice.
It failse because the other project is not contained in the projects (Matchers.containsInAnyOrder(workProject1, workProject2)).
| } | ||
| } | ||
|
|
||
| public List<HeimatTask> getAllKnownHeimatTasks(final LocalDate forDate) { |
There was a problem hiding this comment.
I'm not sure why we need this. It is referenced in some places - but not sure why we need it. need to dig deeper to understand why we need this
There was a problem hiding this comment.
The method gets tasks from the API for that date (via getTasks), and also includes tasks from stored mappings (even if not available today) and merges both.
| private Label heimatExpiresLabel; | ||
|
|
||
| @FXML | ||
| private Label expirationDateLabel; |
There was a problem hiding this comment.
we can write all info into existing heimatExpiresLabel? or why do we need a 2nd label for this?
There was a problem hiding this comment.
ah I see. the label holds the row-name for the setting "Expires:" <value> currently. Still, I would keep this as is and ad the Expired info to the value displayed.
Feels a little uncommon that the value description changes for me :)
| } catch(Exception e){ | ||
| heimatExpiresLabel.setText("Does not seem to be valid"); | ||
| heimatExpiresLabel.setText(""); | ||
| expirationDateLabel.setText("Does not seem to be valid"); |
There was a problem hiding this comment.
could also set it to color red if its not valid
| final ObservableList<HeimatController.ProjectMapping> newProjectMappings = FXCollections.observableArrayList( | ||
| previousProjectMappings); | ||
|
|
||
| Platform.runLater(() -> { |
There was a problem hiding this comment.
runLater with a dialog which modifies the ui 'indirectly' seems not soo nice. not sure if we could solve this differently
There was a problem hiding this comment.
I changed it to use on thisStage.setOnShown, however I don"t know if that is a good idea, so I will leave it but if you want to revert it then feel free to tell me.
|
in sync dialog times which exist in heimat but not in keeptime the projects are not shown in bold. might fix in this pr |
* improve test * improve expiration label * refactor code in ExternalProjectsMapController * fix work stacking when it is not needed
Mapping logic and data integrity:
HeimatController.getTableRows()to ensure that all projects mapped to the same Heimat task are grouped together, even if some projects have no worked time, making the mapping more robust and comprehensive.getAllKnownHeimatTasks()to merge tasks from both API and persisted mappings, ensuring no mapped tasks are lost in the UI.User interface and messaging:
TextFlowcomponents, allowing for richer and more readable feedback in the UI (e.g., bold task names and clearer explanations).ExternalProjectsMapControllerto display a warning dialog for mappings that reference missing Heimat tasks, and to allow the user to clear these mappings if desired.Bug fixes and validation:
FileOpenHelper.openFile()where the file existence check was inverted, ensuring files are only opened if they actually exist and are regular files.pendingRemovalflag toProjectMappingto track mappings that should be removed due to missing Heimat tasks, improving data consistency and user awareness.