Skip to content

Commit

Permalink
Allow users to begin merge request even if the supplying plan has bee…
Browse files Browse the repository at this point in the history 
…n deleted
  • Loading branch information
@AaronPlave
AaronPlave committed Oct 30, 2024
1 parent 77f2024 commit b62fa44
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 33 deletions.
19 changes: 9 additions & 10 deletions src/components/modals/PlanMergeRequestsModal.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,7 @@
}
async function onReviewOrWithdraw(planMergeRequest: PlanMergeRequest) {
if (!planMergeRequest.plan_snapshot_supplying_changes.plan || !planMergeRequest.plan_receiving_changes) {
return;
}
if (planMergeRequest.type === 'incoming') {
if (planMergeRequest.type === 'incoming' && planMergeRequest.plan_receiving_changes) {
planMergeRequest.pending = true;
filteredPlanMergeRequests = [...filteredPlanMergeRequests];
const success = await effects.planMergeBegin(
Expand All @@ -94,7 +90,7 @@
dispatch('close');
goto(`${base}/plans/${planMergeRequest.plan_receiving_changes.id}/merge`);
}
} else if (planMergeRequest.type === 'outgoing') {
} else if (planMergeRequest.type === 'outgoing' && planMergeRequest.plan_snapshot_supplying_changes.plan) {
await effects.planMergeRequestWithdraw(
planMergeRequest.id,
planMergeRequest.plan_snapshot_supplying_changes.plan,
Expand All @@ -121,7 +117,10 @@
}
function hasPermission(planMergeRequest: PlanMergeRequest) {
if (!planMergeRequest.plan_snapshot_supplying_changes.plan || !planMergeRequest.plan_receiving_changes) {
const model =
planMergeRequest.plan_snapshot_supplying_changes.plan?.model || planMergeRequest.plan_receiving_changes?.model;
if (!model) {
return false;
}
Expand All @@ -130,14 +129,14 @@
user,
planMergeRequest.plan_snapshot_supplying_changes.plan,
planMergeRequest.plan_receiving_changes,
planMergeRequest.plan_snapshot_supplying_changes.plan.model,
model,
);
}
return featurePermissions.planBranch.canReviewRequest(
user,
planMergeRequest.plan_snapshot_supplying_changes.plan,
planMergeRequest.plan_receiving_changes,
planMergeRequest.plan_snapshot_supplying_changes.plan.model,
model,
);
}
</script>
Expand Down Expand Up @@ -212,7 +211,7 @@
<PlanMergeRequestStatusBadge status={planMergeRequest.status} />
</div>

{#if planMergeRequest.status === 'pending' && planMergeRequest.plan_snapshot_supplying_changes.plan}
{#if planMergeRequest.status === 'pending'}
<button
on:click={() => onReviewOrWithdraw(planMergeRequest)}
class="st-button secondary"
Expand Down
6 changes: 3 additions & 3 deletions src/utilities/effects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4980,12 +4980,12 @@ const effects = {

async planMergeBegin(
merge_request_id: number,
sourcePlan: PlanForMerging,
sourcePlan: PlanForMerging | undefined,
targetPlan: PlanForMerging,
user: User | null,
): Promise<boolean> {
try {
if (!queryPermissions.PLAN_MERGE_BEGIN(user, sourcePlan, targetPlan, sourcePlan.model)) {
if (!queryPermissions.PLAN_MERGE_BEGIN(user, sourcePlan, targetPlan, targetPlan.model)) {
throwPermissionError('begin a merge');
}

Expand Down Expand Up @@ -5080,7 +5080,7 @@ const effects = {
async planMergeRequestWithdraw(
merge_request_id: number,
sourcePlan: PlanForMerging,
targetPlan: PlanForMerging,
targetPlan: PlanForMerging | undefined,
user: User | null,
): Promise<boolean> {
try {
Expand Down
44 changes: 24 additions & 20 deletions src/utilities/permissions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ function getRolePlanBranchPermission(
queries: string[],
user: User | null,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners,
targetPlan: PlanWithOwners | undefined,
model: Pick<Model, 'owner'>,
): boolean {
if (user && user.rolePermissions) {
Expand All @@ -220,23 +220,27 @@ function getRolePlanBranchPermission(
if (user.rolePermissions != null) {
switch (user.rolePermissions[getFunctionPermission(queryName)]) {
case 'OWNER':
permission = (!sourcePlan || isPlanOwner(user, sourcePlan)) && isPlanOwner(user, targetPlan);
permission =
(!sourcePlan || isPlanOwner(user, sourcePlan)) && (!targetPlan || isPlanOwner(user, targetPlan));
break;
case 'MISSION_MODEL_OWNER':
permission = isUserOwner(user, model);
break;
case 'PLAN_OWNER':
permission = (sourcePlan && isPlanOwner(user, sourcePlan)) || isPlanOwner(user, targetPlan);
permission =
(!!sourcePlan && isPlanOwner(user, sourcePlan)) || (!!targetPlan && isPlanOwner(user, targetPlan));
break;
case 'PLAN_COLLABORATOR':
permission = (sourcePlan && isPlanCollaborator(user, sourcePlan)) || isPlanCollaborator(user, targetPlan);
permission =
(!!sourcePlan && isPlanCollaborator(user, sourcePlan)) ||
(!!targetPlan && isPlanCollaborator(user, targetPlan));
break;
case 'PLAN_OWNER_COLLABORATOR':
permission =
(sourcePlan && isPlanOwner(user, sourcePlan)) ||
(sourcePlan && isPlanCollaborator(user, sourcePlan)) ||
isPlanOwner(user, targetPlan) ||
isPlanCollaborator(user, targetPlan);
(!!sourcePlan && isPlanOwner(user, sourcePlan)) ||
(!!sourcePlan && isPlanCollaborator(user, sourcePlan)) ||
(!!targetPlan && isPlanOwner(user, targetPlan)) ||
(!!targetPlan && isPlanCollaborator(user, targetPlan));
break;
case 'PLAN_OWNER_SOURCE':
permission = !!sourcePlan && isPlanOwner(user, sourcePlan);
Expand All @@ -248,13 +252,13 @@ function getRolePlanBranchPermission(
permission = !!sourcePlan && (isPlanOwner(user, sourcePlan) || isPlanCollaborator(user, sourcePlan));
break;
case 'PLAN_OWNER_TARGET':
permission = isPlanOwner(user, targetPlan);
permission = !!targetPlan && isPlanOwner(user, targetPlan);
break;
case 'PLAN_COLLABORATOR_TARGET':
permission = isPlanCollaborator(user, targetPlan);
permission = !!targetPlan && isPlanCollaborator(user, targetPlan);
break;
case 'PLAN_OWNER_COLLABORATOR_TARGET':
permission = isPlanOwner(user, targetPlan) || isPlanCollaborator(user, targetPlan);
permission = !!targetPlan && (isPlanOwner(user, targetPlan) || isPlanCollaborator(user, targetPlan));
break;
case 'NO_CHECK':
default:
Expand Down Expand Up @@ -725,8 +729,8 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
},
PLAN_MERGE_BEGIN: (
user: User | null,
sourcePlan: PlanWithOwners,
targetPlan: PlanWithOwners,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
): boolean => {
const queries = [Queries.BEGIN_MERGE];
Expand All @@ -738,7 +742,7 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
PLAN_MERGE_CANCEL: (
user: User | null,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
): boolean => {
const queries = [Queries.CANCEL_MERGE];
Expand All @@ -750,7 +754,7 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
PLAN_MERGE_COMMIT: (
user: User | null,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
): boolean => {
const queries = [Queries.COMMIT_MERGE];
Expand All @@ -761,8 +765,8 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
},
PLAN_MERGE_DENY: (
user: User | null,
sourcePlan: PlanWithOwners,
targetPlan: PlanWithOwners,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
): boolean => {
const queries = [Queries.DENY_MERGE];
Expand All @@ -773,8 +777,8 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
},
PLAN_MERGE_REQUEST_WITHDRAW: (
user: User | null,
sourcePlan: PlanWithOwners,
targetPlan: PlanWithOwners,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
): boolean => {
const queries = [Queries.WITHDRAW_MERGE_REQUEST];
Expand Down Expand Up @@ -1195,7 +1199,7 @@ type RolePlanPermissionCheckWithAsset<T = AssetWithOwner> = (
type RolePlanBranchPermissionCheck = (
user: User | null,
sourcePlan: PlanWithOwners | undefined,
targetPlan: PlanWithOwners,
targetPlan: PlanWithOwners | undefined,
model: ModelWithOwner,
) => boolean;

Expand Down

0 comments on commit b62fa44

Please sign in to comment.