-
Notifications
You must be signed in to change notification settings - Fork 56
feat: Admins should be able to save, edit and delete filters in application groups #1153
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
Conversation
| @@ -1,4 +1,4 @@ | |||
| import React, { Suspense, useCallback, useRef, useEffect, useState, useMemo } from 'react' | |||
| import React, { Suspense, useCallback, useRef, useEffect, useState, useMemo, Children } from 'react' | |||
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.
Please remove unused import
| import { CONTEXT_NOT_AVAILABLE_ERROR } from '../../config/constantMessaging' | ||
| import { toast } from 'react-toastify' | ||
| import CreateAppGroup from './CreateAppGroup' | ||
| import { create } from 'domain' |
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.
Please remove unused imports
| const [clickedGroup, setClickedGroup] = useState<GroupOptionType>(null) | ||
| const [allAppsList, setAllAppsList] = useState<CreateGroupAppListType[]>([]) | ||
| const [isVirtualEnv, setIsVirtualEnv] = useState<boolean>(false) | ||
| const [unauthorizedApps, setUnauthorizedApps] = useState<String[]>() |
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.
it should be unAuthorizedApps
| } | ||
|
|
||
| const openCreateGroup = (e, groupId?: string) => { | ||
| const handleToast = (field: string) => { |
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.
rename field to action
| if(err['code'] === 403) { | ||
| let arrUnauthorized = [] | ||
| err['errors'].map((errors) => { | ||
| arrUnauthorized.push([...errors['userMessage']['unauthorizedApps']]) |
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.
you need to update this in the state to show on create page
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.
Updated state stored in hashmap
| setLoading(true) | ||
| try { | ||
| const {result} = await appGroupPermission(envId, payload) | ||
| if(result === true) { |
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.
we can write it as if(result) {
| } | ||
| } | ||
|
|
||
| async function getAllPermissionCheck(payload: CheckPermissionType) { |
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.
It seems replica of above function, can't we merge both?
| import Tippy from '@tippyjs/react' | ||
|
|
||
| export default function CreateAppGroup({ appList, selectedAppGroup, closePopup }: CreateGroupType) { | ||
| export default function CreateAppGroup({ appList, selectedAppGroup, closePopup, unauthorizedApps }: CreateGroupType) { |
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.
rename unauthorizedApps to unAuthorizedApps
| setSelectedAppsCount(_selectedAppsCount) | ||
| } | ||
|
|
||
| const unauthorizedAppCheck = (appName: String) => { |
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.
instead of doing all this we should pass a map of unauthorized apps from parent
|
Kudos, SonarCloud Quality Gate passed!
|








Description
As of now only super admins can do save selection as filter or modify or delete and existing filter group.
This feature request is raised to extend this capability to admins.
Fixes devtron-labs/devtron#3402
Type of change
How Has This Been Tested?
Checklist: