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

avoid listing projects with incomplete grid data [MRXNM-30] #1545

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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
39 changes: 34 additions & 5 deletions api/apps/api/src/modules/projects/projects-crud.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { ProjectId, SetProjectGridFromShapefile } from './planning-unit-grid';
import { ProjectRoles } from '@marxan-api/modules/access-control/projects-acl/dto/user-role-project.dto';
import { Roles } from '@marxan-api/modules/access-control/role.api.entity';
import { PlanningUnitGridShape } from '@marxan/scenarios-planning-unit';
import { PublishedProject } from '../published-project/entities/published-project.api.entity';
import { CostSurfaceId } from '@marxan-api/modules/projects/planning-unit-grid/project.id';
import { getDefaultCostSurfaceIdFromProject } from '@marxan-api/modules/projects/get-default-project-cost-surface';

Expand Down Expand Up @@ -61,8 +60,6 @@ export class ProjectsCrudService extends AppBaseService<
private readonly userProjects: Repository<UsersProjectsApiEntity>,
@InjectRepository(ProtectedArea, DbConnections.geoprocessingDB)
private readonly protectedAreas: Repository<ProtectedArea>,
@InjectRepository(PublishedProject)
private readonly publishedProjects: Repository<PublishedProject>,
private readonly commandBus: CommandBus,
) {
super(repository, 'project', 'projects', {
Expand Down Expand Up @@ -128,6 +125,34 @@ export class ProjectsCrudService extends AppBaseService<
return query;
}

/**
* In some cases, newly-created projects may not have any of the grid-related
* data: either a GADM id (if the project was created using a GADM area as
* planning area) or a planning area geometry id (if the user uploaded a
* shapefile with the project's planning area or planning grid).
*
* This could either be a temporary state (while the planning grid is being
* created in geodb) or an error situation. Either way, allowing users to
* "discover" such projects (e.g. when requesting a list of projects they
* have access to) is not desirable, as no meaningful work can be done when
* a project is in a state like this. Therefore, we hide such projects from
* the results of `findAll()` and `getById()` queries.
*/
private async hideProjectsWithIncompleteGridData(
query: SelectQueryBuilder<Project>,
): Promise<SelectQueryBuilder<Project>> {
query.andWhere(`NOT(
${this.alias}.country_id IS NULL
AND
${this.alias}.admin_area_l1_id IS NULL
AND
${this.alias}.admin_area_l2_id IS NULL
AND
${this.alias}.planning_area_geometry_id IS NULL
)`);
return query;
}

async setDataCreate(
create: CreateProjectDTO,
info?: ProjectsRequest,
Expand Down Expand Up @@ -264,8 +289,8 @@ export class ProjectsCrudService extends AppBaseService<

async extendGetByIdQuery(
query: SelectQueryBuilder<Project>,
fetchSpecification?: FetchSpecification,
info?: ProjectsRequest,
_fetchSpecification?: FetchSpecification,
_info?: ProjectsRequest,
): Promise<SelectQueryBuilder<Project>> {
/**
* Bring in publicMetadata (if the project has been made public). This is
Expand All @@ -274,6 +299,8 @@ export class ProjectsCrudService extends AppBaseService<
*/
query.leftJoinAndSelect('project.publicMetadata', 'publicMetadata');

this.hideProjectsWithIncompleteGridData(query);

return query;
}

Expand Down Expand Up @@ -318,6 +345,8 @@ export class ProjectsCrudService extends AppBaseService<
);
}

this.hideProjectsWithIncompleteGridData(query);

if (loggedUser) {
query
.andWhere(`acl.user_id = :userId`, {
Expand Down
4 changes: 2 additions & 2 deletions api/apps/api/test/e2e.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const E2E_CONFIG: {
name: faker.random.words(5),
organizationId: faker.random.uuid(),
planningUnitGridShape: PlanningUnitGridShape.Hexagon,
planningUnitAreakm2: 10,
planningUnitAreakm2: 10000,
countryId: 'NAM',
}),
minimalInGivenAdminArea: (options?: {
Expand All @@ -135,7 +135,7 @@ export const E2E_CONFIG: {
adminAreaLevel1Id: options?.adminAreaLevel1Id,
adminAreaLevel2Id: options?.adminAreaLevel2Id,
planningUnitGridShape: PlanningUnitGridShape.Hexagon,
planningUnitAreakm2: 10,
planningUnitAreakm2: 10000,
}),
complete: (options: CountryCodeInput): CreateProjectDTO => ({
name: faker.random.words(5),
Expand Down
12 changes: 12 additions & 0 deletions api/apps/api/test/projects/crud/project-get.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,15 @@ test(`getting a project where the user is not in project`, async () => {
);
fixtures.ThenForbiddenIsReturned(response);
});

test(`if a project was created with malformed grid data and not GC'ed yet, it should not be included in project listings`, async () => {
const projectId = await fixtures.GivenPrivateProjectWithMalformedGridDataWasCreated();
const response = await fixtures.WhenGettingUserProjects();
fixtures.ThenProjectIsNotIncludedInProjectsList(projectId, response);
});

test(`if a project was created with malformed grid data and not GC'ed yet, a request to get its information should return 404`, async () => {
const projectId = await fixtures.GivenPrivateProjectWithMalformedGridDataWasCreated();
const response = await fixtures.WhenGettingProject(projectId);
fixtures.ThenNotFoundIsReturned(response);
});
2 changes: 1 addition & 1 deletion api/apps/api/test/projects/crud/update-project.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ test(`updating a project does not work if user is not in project`, async () => {

const request = await fixtures.WhenProjectIsUpdatedAsNotIncludedUser();

await fixtures.ThenForbiddenIsReturned(request);
fixtures.ThenForbiddenIsReturned(request);
});
4 changes: 4 additions & 0 deletions api/apps/api/test/projects/crud/update-project.fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as request from 'supertest';
import { ProjectChecker } from '@marxan-api/modules/projects/project-checker/project-checker.service';
import { ProjectCheckerFake } from '../../utils/project-checker.service-fake';
import { HttpStatus } from '@nestjs/common';
import { PlanningUnitGridShape } from '@marxan/scenarios-planning-unit';

export const getFixtures = async () => {
const app = await bootstrapApplication();
Expand All @@ -30,6 +31,9 @@ export const getFixtures = async () => {
name: originalName,
organizationId,
metadata: {},
countryId: 'RWA',
planningUnitGridShape: PlanningUnitGridShape.Hexagon,
planningUnitAreakm2: 10000,
})
).data.id;
},
Expand Down
34 changes: 34 additions & 0 deletions api/apps/api/test/projects/projects.fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { bootstrapApplication } from '../utils/api-application';
import { EventBusTestUtils } from '../utils/event-bus.test.utils';
import { ScenariosTestUtils } from '../utils/scenarios.test.utils';
import { ScenarioType } from '@marxan-api/modules/scenarios/scenario.api.entity';
import { Project } from '@marxan-api/modules/projects/project.api.entity';

export const getFixtures = async () => {
const app = await bootstrapApplication([CqrsModule], [EventBusTestUtils]);
Expand All @@ -35,6 +36,9 @@ export const getFixtures = async () => {
const publishedProjectsRepo: Repository<PublishedProject> = app.get(
getRepositoryToken(PublishedProject),
);
const projectsRepo: Repository<Project> = app.get(
getRepositoryToken(Project),
);
const cleanups: (() => Promise<void>)[] = [];

const apiEvents = app.get(ApiEventsService);
Expand Down Expand Up @@ -100,6 +104,23 @@ export const getFixtures = async () => {
cleanups.push(cleanup);
return projectId;
},
GivenPrivateProjectWithMalformedGridDataWasCreated: async () => {
const { cleanup, projectId } = await GivenProjectExists(
app,
randomUserToken,
);
cleanups.push(cleanup);
await projectsRepo.update(
{ id: projectId },
{
countryId: undefined,
adminAreaLevel1Id: undefined,
adminAreaLevel2Id: undefined,
planningAreaId: undefined,
},
);
return projectId;
},
GivenScenarioWasCreated: async (projectId: string, name?: string) => {
const result = await ScenariosTestUtils.createScenario(
app,
Expand Down Expand Up @@ -336,10 +357,23 @@ export const getFixtures = async () => {
ThenForbiddenIsReturned: (response: request.Response) => {
expect(response.status).toEqual(403);
},
ThenProjectIsNotIncludedInProjectsList: (
projectId: string,
response: request.Response,
) => {
const allIdsOfProjectsInResponse = response.body.data.map(
(project: { id: string }) => project.id,
);
expect(allIdsOfProjectsInResponse).not.toContain(projectId);
},
WhenGettingProject: async (projectId: string) =>
await request(app.getHttpServer())
.get(`/api/v1/projects/${projectId}`)
.set('Authorization', `Bearer ${randomUserToken}`),
WhenGettingUserProjects: async () =>
await request(app.getHttpServer())
.get(`/api/v1/projects`)
.set('Authorization', `Bearer ${randomUserToken}`),
WhenGettingProjectAsNotIncludedUser: async (projectId: string) =>
await request(app.getHttpServer())
.get(`/api/v1/projects/${projectId}`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { GivenUserIsLoggedIn } from '../../steps/given-user-is-logged-in';
import { OrganizationsTestUtils } from '../../utils/organizations.test.utils';
import { ProjectsTestUtils } from '../../utils/projects.test.utils';
import * as request from 'supertest';
import { PlanningUnitGridShape } from '@marxan/scenarios-planning-unit';

export const getFixtures = async () => {
const app = await bootstrapApplication();
Expand All @@ -16,6 +17,9 @@ export const getFixtures = async () => {
await ProjectsTestUtils.createProject(app, authToken, {
name: `User Project ${Date.now()}`,
organizationId: org.data.id,
countryId: 'RWA',
planningUnitGridShape: PlanningUnitGridShape.Hexagon,
planningUnitAreakm2: 10000,
})
).data.id;
};
Expand Down
Loading