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

fix(geo-features): searching via name should include project-id #572

Merged
merged 3 commits into from
Oct 14, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { GeoFeatureSetSpecification } from './dto/geo-feature-set-specification.
import { GeoFeature } from './geo-feature.api.entity';
import { GeoFeaturePropertySet } from './geo-feature.geo.entity';
import { DbConnections } from '@marxan-api/ormconfig.connections';
import { BBox } from 'geojson';

@Injectable()
export class GeoFeaturePropertySetService {
Expand All @@ -23,24 +24,24 @@ export class GeoFeaturePropertySetService {

getFeaturePropertySetsForFeatures(
geoFeatureIds: string[],
forProject?: Project | null | undefined,
withinBBox?: BBox | null,
): Promise<GeoFeaturePropertySet[]> {
const query = this.geoFeaturePropertySetsRepository
.createQueryBuilder('propertySets')
.distinct(true)
.where(`propertySets.featureId IN (:...ids)`, { ids: geoFeatureIds });

if (forProject) {
if (withinBBox) {
query.andWhere(
`st_intersects(
st_makeenvelope(:xmin, :ymin, :xmax, :ymax, 4326),
"propertySets".bbox
)`,
{
xmin: forProject.bbox[1],
ymin: forProject.bbox[3],
xmax: forProject.bbox[0],
ymax: forProject.bbox[2],
xmin: withinBBox[1],
ymin: withinBBox[3],
xmax: withinBBox[0],
ymax: withinBBox[2],
},
);
}
Expand Down Expand Up @@ -111,7 +112,7 @@ export class GeoFeaturePropertySetService {
Logger.debug(inspect(featuresInSpecification));
const metadataForFeaturesInSpecification = await this.getFeaturePropertySetsForFeatures(
idsOfFeaturesInSpecification,
project,
project?.bbox,
);
const featuresInSpecificationWithPropertiesMetadata = this.extendGeoFeaturesWithPropertiesFromPropertySets(
featuresInSpecification,
Expand Down
11 changes: 11 additions & 0 deletions api/apps/api/src/modules/geo-features/geo-features-request-info.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { AppInfoDTO } from '@marxan-api/dto/info.dto';
import { BBox } from 'geojson';

export interface GeoFeaturesRequestInfo extends AppInfoDTO {
params?: {
featureClassAndAliasFilter?: string;
projectId?: string;
bbox?: BBox;
ids?: string[];
};
}
64 changes: 41 additions & 23 deletions api/apps/api/src/modules/geo-features/geo-features.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { DbConnections } from '@marxan-api/ormconfig.connections';
import { GeometrySource } from './geometry-source.enum';
import { v4 } from 'uuid';
import { UploadShapefileDTO } from '../projects/dto/upload-shapefile.dto';
import { GeoFeaturesRequestInfo } from './geo-features-request-info';

const geoFeatureFilterKeyNames = [
'featureClassName',
Expand All @@ -45,16 +46,12 @@ type GeoFeatureFilterKeys = keyof Pick<
>;
type GeoFeatureFilters = Record<GeoFeatureFilterKeys, string[]>;

type ServiceRequestInfo = AppInfoDTO & {
forProject?: Project;
};

@Injectable()
export class GeoFeaturesService extends AppBaseService<
GeoFeature,
GeoFeatureSetSpecification,
GeoFeatureSetSpecification,
ServiceRequestInfo
GeoFeaturesRequestInfo
> {
constructor(
@InjectRepository(GeoFeatureGeometry, DbConnections.geoprocessingDB)
Expand Down Expand Up @@ -99,7 +96,7 @@ export class GeoFeaturesService extends AppBaseService<
setFilters(
query: SelectQueryBuilder<GeoFeature>,
filters: GeoFeatureFilters,
info?: ServiceRequestInfo,
info?: GeoFeaturesRequestInfo,
): SelectQueryBuilder<GeoFeature> {
this._processBaseFilters<GeoFeatureFilters>(
query,
Expand All @@ -115,7 +112,7 @@ export class GeoFeaturesService extends AppBaseService<
async extendFindAllQuery(
query: SelectQueryBuilder<GeoFeature>,
fetchSpecification: FetchSpecification,
info: ServiceRequestInfo,
info: GeoFeaturesRequestInfo,
): Promise<SelectQueryBuilder<GeoFeature>> {
/**
* We should either list only "public" features (i.e. they are not from a
Expand All @@ -137,15 +134,36 @@ export class GeoFeaturesService extends AppBaseService<
const projectId: string | undefined =
(info?.params?.projectId as string) ??
(fetchSpecification?.filter?.projectId as string);
if (projectId) {
const relatedProject = await this.projectRepository
.findOneOrFail(projectId)
.then((project) => project)
.catch((_error) => {
throw new NotFoundException(
`No project with id ${projectId} exists.`,
);
});

// find over api.features may not be best, as pagination does reflect
// actual page/pageSize vs actual items

// making search via intersection of whole PA vs all features
// may not be best for performance... but ain't we doing it anyway?

// also, current approach may fail as we are using IDs directly (typeorm
// limits - what we cannot overcome unless we duplicate data or make a
// special "cache/view" (per project?)

// current query just 'attaches' 'like' clause in separation of previously
// fetched features (so it may get public ones that are not within study area)

/**
* potential solution but it may be messing much?
*
* 1 keep searching over features_data (intersection) [could be cached
* at some point, per project]
* 2 move api.features into geo.features_data ...
* 3 which also fixes issues with:
* * searching via tag
* * searching via name
* * pagination
* * searching within one query (table) and single db
* * reduces unnecessary relations and system accidental complexity
*
*/

if (projectId && info?.params?.bbox) {
const geoFeaturesWithinProjectBbox = await this.geoFeaturesGeometriesRepository
.createQueryBuilder('geoFeatureGeometries')
.distinctOn(['"geoFeatureGeometries"."feature_id"'])
Expand All @@ -155,10 +173,10 @@ export class GeoFeaturesService extends AppBaseService<
"geoFeatureGeometries".the_geom
)`,
{
xmin: relatedProject.bbox[1],
ymin: relatedProject.bbox[3],
xmax: relatedProject.bbox[0],
ymax: relatedProject.bbox[2],
xmin: info.params.bbox[1],
ymin: info.params.bbox[3],
xmax: info.params.bbox[0],
ymax: info.params.bbox[2],
},
)
.getMany()
Expand Down Expand Up @@ -191,7 +209,7 @@ export class GeoFeaturesService extends AppBaseService<

if (info?.params?.featureClassAndAliasFilter) {
queryFilteredByPublicOrProjectSpecificFeatures.andWhere(
`${this.alias}.alias ilike :featureClassAndAliasFilter OR ${this.alias}.featureClassName ilike :featureClassAndAliasFilter`,
`(${this.alias}.alias ilike :featureClassAndAliasFilter OR ${this.alias}.featureClassName ilike :featureClassAndAliasFilter)`,
{
featureClassAndAliasFilter: `%${info.params.featureClassAndAliasFilter}%`,
},
Expand All @@ -210,7 +228,7 @@ export class GeoFeaturesService extends AppBaseService<
async extendFindAllResults(
entitiesAndCount: [any[], number],
fetchSpecification?: DeepReadonly<FetchSpecification>,
info?: ServiceRequestInfo,
info?: GeoFeaturesRequestInfo,
): Promise<[any[], number]> {
/**
* Short-circuit if there's no result to extend, or if the API client has
Expand Down Expand Up @@ -239,7 +257,7 @@ export class GeoFeaturesService extends AppBaseService<
);

const entitiesWithProperties = await this.geoFeaturesPropertySet
.getFeaturePropertySetsForFeatures(geoFeatureIds, info?.forProject)
.getFeaturePropertySetsForFeatures(geoFeatureIds, info?.params?.bbox)
.then((results) => {
return this.geoFeaturesPropertySet.extendGeoFeaturesWithPropertiesFromPropertySets(
entitiesAndCount[0],
Expand Down
2 changes: 2 additions & 0 deletions api/apps/api/src/modules/geo-features/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { FeaturesCalculated } from './processing';
export { GeoFeaturesService } from './geo-features.service';
export { GeoFeaturesRequestInfo } from './geo-features-request-info';
11 changes: 11 additions & 0 deletions api/apps/api/src/modules/projects/project-requests-info.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { AppInfoDTO } from '@marxan-api/dto/info.dto';
import { BBox } from 'geojson';

export interface ProjectsRequest extends AppInfoDTO {
params?: {
featureClassAndAliasFilter?: string;
projectId?: string;
bbox?: BBox;
nameSearch?: string;
} & AppInfoDTO['params'];
}
29 changes: 12 additions & 17 deletions api/apps/api/src/modules/projects/projects-crud.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import {
} from './planning-areas';
import { UsersProjectsApiEntity } from './control-level/users-projects.api.entity';
import { Roles } from '@marxan-api/modules/users/role.api.entity';
import { AppInfoDTO } from '@marxan-api/dto/info.dto';
import { DbConnections } from '@marxan-api/ormconfig.connections';
import { ProtectedArea } from '@marxan/protected-areas';

import { ProjectsRequest } from './project-requests-info';

const projectFilterKeyNames = [
'name',
'organizationId',
Expand All @@ -41,18 +42,12 @@ type ProjectFilterKeys = keyof Pick<
>;
type ProjectFilters = Record<ProjectFilterKeys, string[]>;

export type ProjectsInfoDTO = AppInfoDTO & {
params?: {
nameSearch?: string;
};
};

@Injectable()
export class ProjectsCrudService extends AppBaseService<
Project,
CreateProjectDTO,
UpdateProjectDTO,
ProjectsInfoDTO
ProjectsRequest
> {
constructor(
@InjectRepository(Project)
Expand Down Expand Up @@ -131,7 +126,7 @@ export class ProjectsCrudService extends AppBaseService<
setFilters(
query: SelectQueryBuilder<Project>,
filters: ProjectFilters,
_info?: ProjectsInfoDTO,
_info?: ProjectsRequest,
): SelectQueryBuilder<Project> {
this._processBaseFilters<ProjectFilters>(
query,
Expand All @@ -143,7 +138,7 @@ export class ProjectsCrudService extends AppBaseService<

async setDataCreate(
create: CreateProjectDTO,
info?: ProjectsInfoDTO,
info?: ProjectsRequest,
): Promise<Project> {
assertDefined(info?.authenticatedUser?.id);
/**
Expand Down Expand Up @@ -180,7 +175,7 @@ export class ProjectsCrudService extends AppBaseService<
async actionAfterCreate(
model: Project,
createModel: CreateProjectDTO,
_info?: ProjectsInfoDTO,
_info?: ProjectsRequest,
): Promise<void> {
if (
createModel.planningUnitAreakm2 &&
Expand Down Expand Up @@ -211,7 +206,7 @@ export class ProjectsCrudService extends AppBaseService<
async actionAfterUpdate(
model: Project,
createModel: UpdateProjectDTO,
_info?: ProjectsInfoDTO,
_info?: ProjectsRequest,
): Promise<void> {
/**
* @deprecated Workers and jobs should be move to the new functionality
Expand Down Expand Up @@ -243,7 +238,7 @@ export class ProjectsCrudService extends AppBaseService<
async setDataUpdate(
model: Project,
update: UpdateProjectDTO,
_?: ProjectsInfoDTO,
_?: ProjectsRequest,
): Promise<Project> {
const bbox = await this.planningAreasService.getPlanningAreaBBox({
...update,
Expand All @@ -262,7 +257,7 @@ export class ProjectsCrudService extends AppBaseService<
async extendGetByIdResult(
entity: Project,
_fetchSpecification?: FetchSpecification,
_info?: ProjectsInfoDTO,
_info?: ProjectsRequest,
): Promise<Project> {
const ids: MultiplePlanningAreaIds = entity;
const idAndName = await this.planningAreasService.getPlanningAreaIdAndName(
Expand All @@ -289,7 +284,7 @@ export class ProjectsCrudService extends AppBaseService<
extendGetByIdQuery(
query: SelectQueryBuilder<Project>,
fetchSpecification?: FetchSpecification,
info?: ProjectsInfoDTO,
info?: ProjectsRequest,
): SelectQueryBuilder<Project> {
const loggedUser = Boolean(info?.authenticatedUser);
query.leftJoin(
Expand All @@ -316,7 +311,7 @@ export class ProjectsCrudService extends AppBaseService<
async extendFindAllQuery(
query: SelectQueryBuilder<Project>,
fetchSpecification: FetchSpecification,
info?: ProjectsInfoDTO,
info?: ProjectsRequest,
): Promise<SelectQueryBuilder<Project>> {
const loggedUser = Boolean(info?.authenticatedUser);

Expand Down Expand Up @@ -382,7 +377,7 @@ export class ProjectsCrudService extends AppBaseService<
async extendFindAllResults(
entitiesAndCount: [Project[], number],
_fetchSpecification?: FetchSpecification,
_info?: ProjectsInfoDTO,
_info?: ProjectsRequest,
): Promise<[Project[], number]> {
const extendedEntities: Promise<Project>[] = entitiesAndCount[0].map(
(entity) => this.extendGetByIdResult(entity),
Expand Down
4 changes: 3 additions & 1 deletion api/apps/api/src/modules/projects/projects.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,14 @@ export class ProjectsController {
@ProcessFetchSpecification() fetchSpecification: FetchSpecification,
@Param() params: { projectId: string },
@Query('q') featureClassAndAliasFilter: string,
@Req() req: RequestWithAuthenticatedUser,
): Promise<GeoFeatureResult> {
const { data, metadata } = await this.projectsService.findAllGeoFeatures(
fetchSpecification,
{
authenticatedUser: req.user,
params: {
...params,
projectId: params.projectId,
featureClassAndAliasFilter: featureClassAndAliasFilter,
},
},
Expand Down
Loading