-
Notifications
You must be signed in to change notification settings - Fork 463
Renaming functionality for Filter-presets #4861
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1717,6 +1717,61 @@ def storeFilterPreset(self, filterpreset): | |
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| "CodeChecker could not store the filter preset: " + str(ex)) | ||
|
|
||
| @exc_to_thrift_reqfail | ||
| @timeit | ||
| def renameFilterPreset(self, preset_id: int, name: str): | ||
| """ | ||
| Rename a filter preset. | ||
| Returns the ID of the renamed preset. | ||
| Raises an error if id/name is empty or if | ||
| a preset with the new name already exists. | ||
| """ | ||
| self.__require_admin() | ||
| try: | ||
| with DBSession(self._Session) as session: | ||
| if not name or not name.strip(): | ||
| raise codechecker_api_shared.ttypes.RequestFailed( | ||
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| "Preset name cannot be empty!") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either use "xyz cannot be empty" or "Invalid xyz" but choose one and stick with it. I'd use "Preset name cannot be empty" and "Preset ID cannot be empty" since this error is thrown because a required argument was left out and so it cannot be invalid if it is empty. |
||
|
|
||
| if not preset_id: | ||
| raise codechecker_api_shared.ttypes.RequestFailed( | ||
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| "Invalid preset ID!") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, add an empty line between validation and the actual query of the filter preset for better readability. |
||
| preset_entry = session.query(FilterPreset).filter( | ||
| FilterPreset.id == preset_id | ||
| ).one_or_none() | ||
|
|
||
| if not preset_entry: | ||
| raise codechecker_api_shared.ttypes.RequestFailed( | ||
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| f"No filter preset found with id {preset_id}!") | ||
|
|
||
| existing = session.query(FilterPreset).filter( | ||
| FilterPreset.preset_name == name | ||
| ).one_or_none() | ||
|
|
||
| if existing and existing.id != preset_id: | ||
| raise codechecker_api_shared.ttypes.RequestFailed( | ||
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| f"A filter preset with name " | ||
| f"'{name}' already exists!") | ||
|
|
||
| LOG.info("Renaming filter preset { id: %d, name: %s } " | ||
| "with name: '%s' ", | ||
| preset_entry.id, preset_entry.preset_name, | ||
| name) | ||
|
|
||
| preset_entry.preset_name = name | ||
| session.commit() | ||
| return preset_entry.id | ||
|
|
||
| except Exception as ex: | ||
| session.rollback() | ||
| raise codechecker_api_shared.ttypes.RequestFailed( | ||
| codechecker_api_shared.ttypes.ErrorCode.DATABASE, | ||
| "CodeChecker could not rename filter preset: " + str(ex)) | ||
|
|
||
| @exc_to_thrift_reqfail | ||
| @timeit | ||
| def deleteFilterPreset(self, preset_id): | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,15 +31,17 @@ | |
| <v-btn | ||
| color="primary" | ||
| :disabled="!presetName" | ||
| @click="saveCurrentFilter(saveMode)" | ||
| @click="saveMode !== 'rename' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd move this logic to the function "saveCurrentFilter()" instead. Also it could be renamed to savePreset and it could handle saving and renaming too. |
||
| ? saveCurrentFilter(saveMode) | ||
| : renameFilterPreset()" | ||
| > | ||
| Save | ||
| </v-btn> | ||
| </v-card-actions> | ||
| </v-card> | ||
| </v-dialog> | ||
| <div> | ||
| <div | ||
| <div | ||
| class="mt-2 mb-2 d-flex align-center justify-space-between w-100 p-1" | ||
| > | ||
| <ClearAllFilters | ||
|
|
@@ -103,6 +105,19 @@ | |
| Create Preset | ||
| </v-btn> | ||
| </div> | ||
| <div | ||
| v-if="canSeeActions && !presetMenuRef?.isModified | ||
| && presetMenuRef?.activePresetId" | ||
| class="d-flex flex-column mb-2 mt-2" | ||
| > | ||
| <v-btn | ||
| color="primary" | ||
| class="mb-2" | ||
| @click="renamePresetDialog" | ||
| > | ||
| Rename | ||
| </v-btn> | ||
| </div> | ||
| <div | ||
| v-if="canSeeActions && presetMenuRef?.isModified" | ||
| class="d-flex flex-column mb-2 mt-2" | ||
|
|
@@ -497,13 +512,14 @@ const saveDialogTitle = computed(() => { | |
| create: "Create new preset", | ||
| override: "Override existing preset", | ||
| createNew: "Save as new preset", | ||
| rename: "Rename the preset" | ||
| }; | ||
| return titles[saveMode.value] || "Save filter preset"; | ||
| }); | ||
|
|
||
| watch(() => props.refreshFilter, state => { | ||
| if (!state) return; | ||
|
|
||
| initByUrl(); | ||
| emit("set-refresh-filter-state", false); | ||
| }); | ||
|
|
@@ -685,6 +701,30 @@ function saveCurrentFilter(mode) { | |
| ); | ||
| } | ||
|
|
||
| function renameFilterPreset() { | ||
| const preset_id = presetMenuRef.value?.activePresetId; | ||
| const new_name = presetName.value; | ||
| new Promise( | ||
| resolve => { | ||
| ccService.getClient().renameFilterPreset(preset_id, new_name, | ||
| handleThriftError(result => { | ||
| resolve(result); | ||
| }) | ||
| ); | ||
| }) | ||
| .then( | ||
| result => { | ||
| savePresetDialogOpen.value = false; | ||
| presetName.value = ""; | ||
| presetMenuRef.value?.selectPresetAfterSave(result); | ||
| } | ||
| ).catch( | ||
| err => { | ||
| handleThriftError("FAILURE", err); | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| function overridePreset() { | ||
| saveMode.value = "override"; | ||
| savePresetDialogOpen.value = true; | ||
|
|
@@ -701,6 +741,10 @@ function createPreset() { | |
| savePresetDialogOpen.value = true; | ||
| } | ||
|
|
||
| function renamePresetDialog() { | ||
| saveMode.value = "rename"; | ||
| savePresetDialogOpen.value = true; | ||
| } | ||
| /*function deletePreset(preset_id) { | ||
| new Promise(resolve => { | ||
| ccService.getClient().deleteFilterPreset(preset_id, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are supplying the preset_id which you want to rename, it is not much information to return it at the end too. I'd return a success instead like a 0 and raise (as currently implemented) if there was any error during. What was the motivation behind returning the preset_id?