Skip to content
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

adding update consumption role capability #1020

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions backend/dataall/core/environment/api/input_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,11 @@ class EnvironmentSortField(GraphQLEnumMapper):
gql.Argument(name='groupUri', type=gql.String),
],
)

UpdateConsumptionRoleInput = gql.InputType(
name='UpdateConsumptionRoleInput',
arguments=[
gql.Argument('consumptionRoleName', gql.String),
gql.Argument('groupUri', gql.String),
],
)
14 changes: 13 additions & 1 deletion backend/dataall/core/environment/api/mutations.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
NewEnvironmentInput,
EnableDataSubscriptionsInput,
InviteGroupOnEnvironmentInput,
AddConsumptionRoleToEnvironmentInput
AddConsumptionRoleToEnvironmentInput,
UpdateConsumptionRoleInput,
)
from dataall.core.environment.api.resolvers import *

Expand Down Expand Up @@ -110,3 +111,14 @@
resolver=disable_subscriptions,
type=gql.Boolean,
)

updateConsumptionRole = gql.MutationField(
name='updateConsumptionRole',
args=[
gql.Argument('environmentUri', type=gql.NonNullableType(gql.String)),
gql.Argument('consumptionRoleUri', type=gql.NonNullableType(gql.String)),
gql.Argument('input', type=UpdateConsumptionRoleInput),
],
type=gql.Ref('ConsumptionRole'),
resolver=update_consumption_role,
)
11 changes: 11 additions & 0 deletions backend/dataall/core/environment/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,17 @@ def remove_consumption_role(context: Context, source, environmentUri=None, consu
return status


def update_consumption_role(context: Context, source, environmentUri=None, consumptionRoleUri=None, input={}):
with context.engine.scoped_session() as session:
status = EnvironmentService.update_consumption_role(
session=session,
uri=consumptionRoleUri,
env_uri=environmentUri,
input=input,
)
return status
petrkalos marked this conversation as resolved.
Show resolved Hide resolved


def list_environment_invited_groups(
context: Context, source, environmentUri=None, filter=None
):
Expand Down
30 changes: 28 additions & 2 deletions backend/dataall/core/environment/services/environment_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,32 @@ def remove_consumption_role(session, uri, env_uri):
)
return True

@staticmethod
@has_tenant_permission(permissions.MANAGE_ENVIRONMENTS)
@has_resource_permission(permissions.REMOVE_ENVIRONMENT_CONSUMPTION_ROLE)
def update_consumption_role(session, uri, env_uri, input):
role_query = session.query(ConsumptionRole).filter(
(
and_(
ConsumptionRole.consumptionRoleUri == uri,
ConsumptionRole.environmentUri == env_uri,
Copy link
Contributor

@dlpzx dlpzx Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The consumptionRoleUri is unique, we do not need the environmentUri filter, which means that in the mutation we can just have consumptionUri and updateConsumptionUriInput as inputs, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, I will remove it from the entire stack

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually are we sure it's unique? get_environment_consumption_role doesn't make that assumption? I think I will reuse this method as well (I should have anyway) for safety

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call - let's re-use that method get_environment_consumption_role

)
)
)
consumption_role = role_query.first()
if consumption_role:
ResourcePolicy.update_resource_policy(
session=session,
resource_uri=uri,
resource_type=ConsumptionRole.__name__,
old_group=consumption_role.groupUri,
new_group=input['groupUri'],
new_permissions=permissions.CONSUMPTION_ROLE_ALL
)
role_query.update(input)
session.commit()
return role_query.first()

@staticmethod
def query_user_environments(session, username, groups, filter) -> Query:
query = (
Expand Down Expand Up @@ -699,7 +725,7 @@ def query_user_environment_consumption_roles(session, groups, uri, filter) -> Qu
ConsumptionRole.groupUri == group,
)
)
return query
return query.order_by(ConsumptionRole.consumptionRoleUri)

@staticmethod
@has_resource_permission(permissions.LIST_ENVIRONMENT_CONSUMPTION_ROLES)
Expand Down Expand Up @@ -731,7 +757,7 @@ def query_all_environment_consumption_roles(session, uri, filter) -> Query:
ConsumptionRole.groupUri == group,
)
)
return query
return query.order_by(ConsumptionRole.consumptionRoleUri)

@staticmethod
@has_resource_permission(permissions.LIST_ENVIRONMENT_CONSUMPTION_ROLES)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,29 @@ def find_resource_policy(
)
return resource_policy

@staticmethod
def update_resource_policy(
session,
resource_uri: str,
resource_type: str,
old_group: str,
new_group: str,
new_permissions: [str]
) -> models.ResourcePolicy:
ResourcePolicy.delete_resource_policy(
session=session,
group=old_group,
resource_uri=resource_uri,
resource_type=resource_type,
)
return ResourcePolicy.attach_resource_policy(
session=session,
group=new_group,
resource_uri=resource_uri,
permissions=new_permissions,
resource_type=resource_type,
)

@staticmethod
def attach_resource_policy(
session,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,53 +12,18 @@ import {
import { Formik } from 'formik';
import { useSnackbar } from 'notistack';
import PropTypes from 'prop-types';
import React, { useEffect, useState } from 'react';
import React from 'react';
import * as Yup from 'yup';
import { Defaults } from 'design';
import { SET_ERROR, useDispatch } from 'globalErrors';
import { listEnvironmentGroups, useClient } from 'services';
import { useClient } from 'services';
import { addConsumptionRoleToEnvironment } from '../services';
import { useFetchGroups } from '../../../utils/api';
petrkalos marked this conversation as resolved.
Show resolved Hide resolved

export const EnvironmentRoleAddForm = (props) => {
const { environment, onClose, open, reloadRoles, ...other } = props;
const { enqueueSnackbar } = useSnackbar();
const dispatch = useDispatch();
const client = useClient();
const [loadingGroups, setLoadingGroups] = useState(true);
const [groupOptions, setGroupOptions] = useState([]);

const fetchGroups = async (environmentUri) => {
try {
setLoadingGroups(true);
const response = await client.query(
listEnvironmentGroups({
filter: Defaults.selectListFilter,
environmentUri
})
);
if (!response.errors) {
setGroupOptions(
response.data.listEnvironmentGroups.nodes.map((g) => ({
value: g.groupUri,
label: g.groupUri
}))
);
} else {
dispatch({ type: SET_ERROR, error: response.errors[0].message });
}
} catch (e) {
dispatch({ type: SET_ERROR, error: e.message });
} finally {
setLoadingGroups(false);
}
};

useEffect(() => {
if (client && environment) {
fetchGroups(environment.environmentUri).catch((e) =>
dispatch({ type: SET_ERROR, error: e.message })
);
}
}, [client, environment, dispatch]);

async function submit(values, setStatus, setSubmitting, setErrors) {
try {
Expand Down Expand Up @@ -98,6 +63,8 @@ export const EnvironmentRoleAddForm = (props) => {
}
}

let { groupOptions, loadingGroups } = useFetchGroups(environment);

if (!environment) {
return null;
}
Expand Down
Loading
Loading