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

feat: Move model from one project to another #1057

Merged
merged 1 commit into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 16 additions & 6 deletions backend/capellacollab/projects/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

@router.get("", response_model=list[models.Project], tags=["Projects"])
def get_projects(
minimum_role: projects_users_models.ProjectUserRole | None = None,
user: users_models.DatabaseUser = fastapi.Depends(
users_injectables.get_own_user
),
Expand All @@ -57,12 +58,21 @@ def get_projects(
log.debug("Fetching all projects")
return list(crud.get_projects(db))

projects = [
association.project
for association in user.projects
if not association.project.visibility == models.Visibility.INTERNAL
]
projects.extend(crud.get_internal_projects(db))
if not minimum_role:
projects = [
association.project
for association in user.projects
if not association.project.visibility == models.Visibility.INTERNAL
]
projects.extend(crud.get_internal_projects(db))
else:
projects = [
association.project
for association in user.projects
if auth_injectables.ProjectRoleVerification(
minimum_role, verify=False
)(association.project.slug, username, db)
]

log.debug("Fetching the following projects: %s", projects)
return projects
Expand Down
2 changes: 2 additions & 0 deletions backend/capellacollab/projects/toolmodels/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,11 @@ def update_model(
name: str | None,
version: tools_models.DatabaseVersion,
nature: tools_models.DatabaseNature,
project: projects_model.DatabaseProject,
) -> models.DatabaseCapellaModel:
model.version = version
model.nature = nature
model.project = project
if description:
model.description = description
if name:
Expand Down
5 changes: 3 additions & 2 deletions backend/capellacollab/projects/toolmodels/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ class PostCapellaModel(pydantic.BaseModel):
class PatchCapellaModel(pydantic.BaseModel):
name: str | None = None
description: str | None = None
version_id: int
nature_id: int
version_id: int | None = None
nature_id: int | None = None
project_slug: str | None = None


class ToolDetails(pydantic.BaseModel):
Expand Down
69 changes: 64 additions & 5 deletions backend/capellacollab/projects/toolmodels/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from capellacollab.tools import crud as tools_crud
from capellacollab.tools import injectables as tools_injectables
from capellacollab.tools import models as tools_models
from capellacollab.users import injectables as users_injectables
from capellacollab.users import models as users_models

from . import crud, injectables, models, workspace
from .backups import routes as backups_routes
Expand Down Expand Up @@ -129,6 +131,9 @@
injectables.get_existing_capella_model
),
db: orm.Session = fastapi.Depends(database.get_db),
user: users_models.DatabaseUser = fastapi.Depends(
users_injectables.get_own_user
),
) -> models.DatabaseCapellaModel:
if body.name:
new_slug = slugify.slugify(body.name)
Expand All @@ -143,26 +148,42 @@
},
)

version = get_version_by_id_or_raise(db, body.version_id)
if version.tool != model.tool:
version = (
get_version_by_id_or_raise(db, body.version_id)
if body.version_id
else model.version
)
if body.version_id and version.tool != model.tool:
raise fastapi.HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail={
"reason": f"The tool having the version “{version.name}” (“{version.tool.name}”) does not match the tool of the model “{model.name}” (“{model.tool.name}”)."
},
)

nature = get_nature_by_id_or_raise(db, body.nature_id)
if nature.tool != model.tool:
nature = (
get_nature_by_id_or_raise(db, body.nature_id)
if body.nature_id
else model.nature
)
if body.nature_id is not None and nature.tool != model.tool:
raise fastapi.HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail={
"reason": f"The tool having the nature “{nature.name}” (“{nature.tool.name}”) does not match the tool of the model “{model.name}” (“{model.tool.name}”)."
},
)

if body.project_slug:
new_project = determine_new_project_to_move_model(
body.project_slug, db, user
)
raise_if_model_exists_in_project(model, new_project)
else:
new_project = model.project

return crud.update_model(
db, model, body.description, body.name, version, nature
db, model, body.description, body.name, version, nature, new_project
)


Expand Down Expand Up @@ -235,6 +256,44 @@
)


def determine_new_project_to_move_model(
project_slug: str, db: orm.Session, user: users_models.DatabaseUser
) -> projects_models.DatabaseProject:
new_project = projects_injectables.get_existing_project(project_slug, db)
success = user.role == users_models.Role.ADMIN
for association in user.projects:
if association.project_id == new_project.id:
if (
not association.role
== projects_users_models.ProjectUserRole.MANAGER
):
break

Check warning on line 270 in backend/capellacollab/projects/toolmodels/routes.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/projects/toolmodels/routes.py#L270

Added line #L270 was not covered by tests
else:
success = True

if not success:
raise fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"reason": f"Requesting user does not have permission to move toolmodel to {new_project.slug}"
},
)
return new_project


def raise_if_model_exists_in_project(
model: models.DatabaseCapellaModel,
project: projects_models.DatabaseProject,
):
if model.slug in [model.slug for model in project.models]:
raise fastapi.HTTPException(

Check warning on line 289 in backend/capellacollab/projects/toolmodels/routes.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/projects/toolmodels/routes.py#L289

Added line #L289 was not covered by tests
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"reason": f"Model with name {model.name} already exists in project {project.slug}"
},
)


router.include_router(
modelsources_routes.router,
prefix="/{model_slug}/modelsources",
Expand Down
31 changes: 31 additions & 0 deletions backend/tests/projects/test_projects_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,34 @@ def test_delete_pipeline_called_when_archiving_project(
mock_delete_pipeline.assert_called_once_with(
db, mock_pipeline, mock.ANY, True
)


@pytest.mark.usefixtures("project_user")
def test_get_project_per_role_user(client: testclient.TestClient):
response = client.get("/api/v1/projects/?minimum_role=user")
assert response.status_code == 200
assert len(response.json()) > 0


@pytest.mark.usefixtures("project_user")
def test_get_project_per_role_manager_as_user(client: testclient.TestClient):
response = client.get("/api/v1/projects/?minimum_role=manager")
assert response.status_code == 200
assert len(response.json()) == 0


@pytest.mark.usefixtures("project_manager")
def test_get_project_per_role_manager(client: testclient.TestClient):
response = client.get("/api/v1/projects/?minimum_role=manager")
assert response.status_code == 200
assert len(response.json()) > 0


def test_get_project_per_role_admin(
client: testclient.TestClient, executor_name: str, db: orm.Session
):
users_crud.create_user(db, executor_name, users_models.Role.ADMIN)

response = client.get("/api/v1/projects/?minimum_role=administrator")
assert response.status_code == 200
assert len(response.json()) > 0
59 changes: 59 additions & 0 deletions backend/tests/projects/toolmodels/test_toolmodels.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# SPDX-FileCopyrightText: Copyright DB Netz AG and the capella-collab-manager contributors
# SPDX-License-Identifier: Apache-2.0

from uuid import uuid4

import pytest
from fastapi import testclient
from sqlalchemy import orm

import capellacollab.projects.users.crud as projects_users_crud
import capellacollab.projects.users.models as projects_users_models
from capellacollab.projects import crud as projects_crud
from capellacollab.projects import models as projects_models
from capellacollab.projects.toolmodels import models as toolmodels_models
from capellacollab.users import models as users_models


def test_move_toolmodel(
project: projects_models.DatabaseProject,
project_manager: users_models.DatabaseUser,
capella_model: toolmodels_models.CapellaModel,
client: testclient.TestClient,
db: orm.Session,
):
second_project = projects_crud.create_project(db, str(uuid4()))
projects_users_crud.add_user_to_project(
db,
project=second_project,
user=project_manager,
role=projects_users_models.ProjectUserRole.MANAGER,
permission=projects_users_models.ProjectUserPermission.WRITE,
)

response = client.patch(
f"/api/v1/projects/{project.slug}/models/{capella_model.slug}",
json={"project_slug": second_project.slug},
)
assert response.status_code == 200

response = client.get(
f"/api/v1/projects/{second_project.slug}/models/{capella_model.slug}"
)
assert response.status_code == 200


@pytest.mark.usefixtures("project_manager")
def test_move_toolmodel_non_project_member(
project: projects_models.DatabaseProject,
capella_model: toolmodels_models.CapellaModel,
client: testclient.TestClient,
db: orm.Session,
):
second_project = projects_crud.create_project(db, str(uuid4()))

response = client.patch(
f"/api/v1/projects/{project.slug}/models/{capella_model.slug}",
json={"project_slug": second_project.slug},
)
assert response.status_code == 401
2 changes: 2 additions & 0 deletions frontend/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import { ModelWrapperComponent } from './projects/models/model-wrapper/model-wra
import { EditProjectMetadataComponent } from './projects/project-detail/edit-project-metadata/edit-project-metadata.component';
import { ModelComplexityBadgeComponent } from './projects/project-detail/model-overview/model-complexity-badge/model-complexity-badge.component';
import { ModelOverviewComponent } from './projects/project-detail/model-overview/model-overview.component';
import { MoveModelComponent } from './projects/project-detail/model-overview/move-model/move-model.component';
import { ProjectDetailsComponent } from './projects/project-detail/project-details.component';
import { ProjectMetadataComponent } from './projects/project-detail/project-metadata/project-metadata.component';
import { AddUserToProjectDialogComponent } from './projects/project-detail/project-users/add-user-to-project/add-user-to-project.component';
Expand Down Expand Up @@ -192,6 +193,7 @@ import { SettingsComponent } from './settings/settings.component';
ModelOverviewComponent,
ModelRestrictionsComponent,
ModelWrapperComponent,
MoveModelComponent,
NavBarMenuComponent,
NoticeComponent,
PipelineRunWrapperComponent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class ModelDescriptionComponent implements OnInit {
onSubmit(): void {
if (this.form.value && this.modelSlug && this.projectSlug) {
this.modelService
.updateModelDescription(this.projectSlug, this.modelSlug, {
.updateModel(this.projectSlug, this.modelSlug, {
name: this.form.value.name || undefined,
description: this.form.value.description || '',
nature_id: this.form.value.nature || undefined,
Expand Down
20 changes: 19 additions & 1 deletion frontend/src/app/projects/models/service/model.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class ModelService {
);
}

updateModelDescription(
updateModel(
projectSlug: string,
modelSlug: string,
patchModel: PatchModel,
Expand Down Expand Up @@ -147,6 +147,23 @@ export class ModelService {
);
};
}

moveModelToProject(
projectSlug: string,
modelSlug: string,
project_slug: string,
): Observable<Model> {
return this.http
.patch<Model>(`${this.backendURLFactory(projectSlug, modelSlug)}/move`, {
project_slug,
})
.pipe(
tap(() => {
this.loadModels(projectSlug);
this._model.next(undefined);
}),
);
}
}

export type NewModel = {
Expand Down Expand Up @@ -174,6 +191,7 @@ export type PatchModel = {
description?: string;
nature_id?: number;
version_id?: number;
project_slug?: string;
};

export function getPrimaryGitModel(model: Model): GetGitModel | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ <h2>Models</h2>
>
<mat-icon>key</mat-icon>
</a>
<button
Paula-Kli marked this conversation as resolved.
Show resolved Hide resolved
mat-mini-fab
matTooltip="Move model to different project"
class="!m-1.5"
(click)="openMoveToProjectDialog(model)"
*ngIf="projectUserService.verifyRole('manager')"
>
<mat-icon>drive_file_move</mat-icon>
</button>
<a
mat-mini-fab
matTooltip="Configure model sources"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Model,
ModelService,
} from 'src/app/projects/models/service/model.service';
import { MoveModelComponent } from 'src/app/projects/project-detail/model-overview/move-model/move-model.component';
import { ProjectUserService } from 'src/app/projects/project-detail/project-users/service/project-user.service';
import { UserService } from 'src/app/services/user/user.service';
import { SessionService } from 'src/app/sessions/service/session.service';
Expand Down Expand Up @@ -79,4 +80,12 @@ export class ModelOverviewComponent implements OnInit {
const primaryModel = getPrimaryGitModel(model);
return primaryModel ? primaryModel.path : '';
}

openMoveToProjectDialog(model: Model): void {
this.dialog.open(MoveModelComponent, {
maxWidth: '100vw',
maxHeight: '200vw',
data: { projectSlug: this.project?.slug, model: model },
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/*
* SPDX-FileCopyrightText: Copyright DB Netz AG and the capella-collab-manager contributors
* SPDX-License-Identifier: Apache-2.0
*/
Loading
Loading