Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Commit

Permalink
Cf validation fix following v3 changes (#4080)
Browse files Browse the repository at this point in the history
* Fix issue where clicking on org and then app fav failed
    - Issue 1 (slightly unrelated)
      - app service getspace action used space schema without an org
    - Issue 2
      - app entity validation found it was missing the space
      - validation process fetched space with custom action
      - custom action did not contain schema key linked to space schema with org
      - this lead to org being not stored correctly in store (contained in space rather than seperatly)
      - Simple fix (see 0a16203284c for harder)
         - When normalizing prioritise the action's schema over attempting
           to fetch via entity catalogue + schemaKey
           (avoids A LOT of plumbing)

* Favourites - Remove duplicated space name, add creation to space + org

* Remove duplicate space

* Remove favourite card double spacing and incorrect/incorreclty tagged 'todo's

Co-authored-by: Neil MacDougall <nwmac@users.noreply.github.com>
  • Loading branch information
richard-cox and nwmac committed Jan 8, 2020
1 parent 468e64d commit 7c94858
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 72 deletions.
13 changes: 9 additions & 4 deletions src/frontend/packages/cloud-foundry/src/cf-entity-generator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as moment from 'moment';

import {
IService,
IServiceBinding,
Expand Down Expand Up @@ -39,6 +40,7 @@ import {
JetstreamError,
} from '../../store/src/entity-request-pipeline/entity-request-base-handlers/handle-multi-endpoints.pipe';
import { JetstreamResponse } from '../../store/src/entity-request-pipeline/entity-request-pipeline.types';
import { EntitySchema } from '../../store/src/helpers/entity-schema';
import { endpointDisconnectRemoveEntitiesReducer } from '../../store/src/reducers/endpoint-disconnect-application.reducer';
import { APIResource } from '../../store/src/types/api.types';
import { IFavoriteMetadata } from '../../store/src/types/user-favorites.types';
Expand Down Expand Up @@ -129,8 +131,6 @@ import { AppStat } from './store/types/app-metadata.types';
import { CFResponse } from './store/types/cf-api.types';
import { GitBranch, GitCommit, GitRepo } from './store/types/git.types';
import { CfUser } from './store/types/user.types';
import { CfApplicationState } from './store/types/application.types';
import { EntitySchema } from '../../store/src/helpers/entity-schema';

export interface CFBasePipelineRequestActionMeta {
includeRelations?: string[];
Expand Down Expand Up @@ -895,7 +895,7 @@ function generateCfApplicationEntity(endpointDefinition: StratosEndpointExtensio
getLink: metadata => `/applications/${metadata.cfGuid}/${metadata.guid}/summary`,
getGuid: metadata => metadata.guid,
getLines: () => ([
['Creation Date', (meta) => meta.createdAt]
['Creation Date', (meta) => meta.createdAt]
])
},
actionBuilders: applicationActionBuilder
Expand Down Expand Up @@ -929,9 +929,10 @@ function generateCfSpaceEntity(endpointDefinition: StratosEndpointExtensionDefin
orgGuid: space.entity.organization_guid ? space.entity.organization_guid : space.entity.organization.metadata.guid,
name: space.entity.name,
cfGuid: space.entity.cfGuid,
createdAt: moment(space.metadata.created_at).format('LLL'),
}),
getLines: () => ([
['Name', (meta) => meta.name],
['Creation Date', (meta) => meta.createdAt]
]),
getLink: metadata => `/cloud-foundry/${metadata.cfGuid}/organizations/${metadata.orgGuid}/spaces/${metadata.guid}/summary`,
getGuid: metadata => metadata.guid
Expand Down Expand Up @@ -964,8 +965,12 @@ function generateCfOrgEntity(endpointDefinition: StratosEndpointExtensionDefinit
status: getOrgStatus(org),
name: org.entity.name,
cfGuid: org.entity.cfGuid,
createdAt: moment(org.metadata.created_at).format('LLL'),
}),
getLink: metadata => `/cloud-foundry/${metadata.cfGuid}/organizations/${metadata.guid}`,
getLines: () => ([
['Creation Date', (meta) => meta.createdAt]
]),
getGuid: metadata => metadata.guid
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { EntitySchema } from '../../../store/src/helpers/entity-schema';
import { entityCatalogue } from '../../../core/src/core/entity-catalogue/entity-catalogue.service';
import { EntitySchema } from '../../../store/src/helpers/entity-schema';

/**
* A structure which represents the tree like layout of entity dependencies. For example organization --> space --> routes
Expand All @@ -18,7 +18,7 @@ export class EntityTreeRelation {

/**
* Creates an instance of EntityTreeRelation.
* @param [isArray=false] is this a collection of entities (should be paginationed) or not
* @param [isArray=false] is this a collection of entities (should be paginated) or not
* @param paramName parameter name of the entity within the schema. For example `space` may be `spaces` (entity.spaces)
* @param [path=''] location of the entity within the parent. For example `space` entity maybe be `entity.spaces`
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import {
routeEntityType,
serviceBindingEntityType,
spaceEntityType,
spaceWithOrgEntityType,
stackEntityType,
} from '../../../../cloud-foundry/src/cf-entity-types';
import { selectCfEntity } from '../../../../cloud-foundry/src/store/selectors/api.selectors';
import { IApp, IAppSummary, IDomain, IOrganization, ISpace } from '../../../../core/src/core/cf-api.types';
import { entityCatalogue } from '../../../../core/src/core/entity-catalogue/entity-catalogue.service';
import { EntityService } from '../../../../core/src/core/entity-service';
Expand All @@ -47,6 +47,7 @@ import { selectUpdateInfo } from '../../../../store/src/selectors/api.selectors'
import { endpointEntitiesSelector } from '../../../../store/src/selectors/endpoint.selectors';
import { APIResource, EntityInfo } from '../../../../store/src/types/api.types';
import { PaginatedAction, PaginationEntityState } from '../../../../store/src/types/pagination.types';
import { cfEntityFactory } from '../../cf-entity-factory';
import { createEntityRelationKey } from '../../entity-relations/entity-relations.types';
import { AppStat } from '../../store/types/app-metadata.types';
import {
Expand All @@ -59,13 +60,13 @@ export function createGetApplicationAction(guid: string, endpointGuid: string) {
return new GetApplication(
guid,
endpointGuid, [
createEntityRelationKey(applicationEntityType, routeEntityType),
createEntityRelationKey(applicationEntityType, spaceEntityType),
createEntityRelationKey(applicationEntityType, stackEntityType),
createEntityRelationKey(applicationEntityType, serviceBindingEntityType),
createEntityRelationKey(routeEntityType, domainEntityType),
createEntityRelationKey(spaceEntityType, organizationEntityType),
]
createEntityRelationKey(applicationEntityType, routeEntityType),
createEntityRelationKey(applicationEntityType, spaceEntityType),
createEntityRelationKey(applicationEntityType, stackEntityType),
createEntityRelationKey(applicationEntityType, serviceBindingEntityType),
createEntityRelationKey(routeEntityType, domainEntityType),
createEntityRelationKey(spaceEntityType, organizationEntityType),
]
);
}

Expand Down Expand Up @@ -177,6 +178,8 @@ export class ApplicationService {
app.cfGuid,
{ includeRelations: [createEntityRelationKey(spaceEntityType, organizationEntityType)], populateMissing: true }
);
getSpaceAction.entity = cfEntityFactory(spaceWithOrgEntityType);
getSpaceAction.schemaKey = spaceWithOrgEntityType;
return this.entityServiceFactory.create<APIResource<ISpace>>(
app.space_guid,
getSpaceAction
Expand All @@ -189,13 +192,9 @@ export class ApplicationService {
);
this.appOrg$ = moreWaiting$.pipe(
first(),
switchMap(app => this.appSpace$.pipe(
map(space => space.entity.organization_guid),
switchMap(orgGuid => {
return this.store.select(selectCfEntity(organizationEntityType, orgGuid));
}),
filter(org => !!org)
))
switchMap(() => this.appSpace$),
map(space => space.entity.organization),
filter(org => !!org)
);

this.isDeletingApp$ = this.appEntityService.isDeletingEntity$.pipe(publishReplay(1), refCount());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { ActionReducer, Store } from '@ngrx/store';
import { normalize } from 'normalizr';

import { AppState, IRequestEntityTypeState } from '../../../../store/src/app-state';
import { EntitySchema } from '../../../../store/src/helpers/entity-schema';
import { NormalizedResponse } from '../../../../store/src/types/api.types';
import { EndpointModel } from '../../../../store/src/types/endpoint.types';
import { APISuccessOrFailedAction, EntityRequestAction } from '../../../../store/src/types/request.types';
import { IEndpointFavMetadata } from '../../../../store/src/types/user-favorites.types';
Expand Down Expand Up @@ -89,8 +87,8 @@ export class StratosBaseCatalogueEntity<
}
return newSchema;
}, {
default: entitySchemas.default
});
default: entitySchemas.default
});
}

private getEndpointType(definition: IStratosBaseEntityDefinition) {
Expand Down Expand Up @@ -217,13 +215,6 @@ export class StratosBaseCatalogueEntity<

}

public getNormalizedEntityData(entities: Y | Y[], schemaKey?: string): NormalizedResponse<Y> {
const schema = this.getSchema(schemaKey);
if (Array.isArray(entities)) {
return normalize(entities, [schema]);
}
return normalize(entities, schema);
}
}

export class StratosCatalogueEntity<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CoreModule } from '../../../../../core/core.module';
import { SharedModule } from '../../../../shared.module';
import { GithubCommitsListConfigServiceBase } from './github-commits-list-config-base.service';

// TODO: RC Move this file to cf package - #3769
// TODO: Move this file to cf package - #3769
describe('GithubCommitsListConfigService', () => {
beforeEach(() => {
TestBed.configureTestingModule({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Action } from '@ngrx/store';
import { normalize } from 'normalizr';

import { StratosBaseCatalogueEntity } from '../../../../core/src/core/entity-catalogue/entity-catalogue-entity';
import { entityCatalogue } from '../../../../core/src/core/entity-catalogue/entity-catalogue.service';
Expand All @@ -10,9 +11,8 @@ import { PipelineResult } from '../entity-request-pipeline.types';
import { getSuccessMapper } from '../pipeline-helpers';
import { endpointErrorsHandlerFactory } from './endpoint-errors.handler';
import { patchActionWithForcedConfig } from './forced-action-type.helpers';
import { HandledMultiEndpointResponse, JetstreamError } from './handle-multi-endpoints.pipe';
import { HandledMultiEndpointResponse, JetstreamError, MultiEndpointResponse } from './handle-multi-endpoints.pipe';
import { multiEndpointResponseMergePipe } from './merge-multi-endpoint-data.pipe';
import { normalizeEntityPipeFactory } from './normalize-entity-request-response.pipe';

const baseErrorHandler = () => 'Api Request Failed';

Expand Down Expand Up @@ -56,19 +56,26 @@ function getEntities(
}, {});
}

// TODO: Type the output of this pipe. #3976
function getNormalizedEntityData(
entities: any[],
action: EntityRequestAction,
catalogueEntity: StratosBaseCatalogueEntity) {
// Can patchActionWithForcedConfig be done outside of the pipe?
// This pipe shouldn't have to worry about the multi entity lists.
const patchedAction = patchActionWithForcedConfig(action);
const schema = patchedAction.entity || catalogueEntity.getSchema(patchedAction.schemaKey);
const arraySafeSchema = Array.isArray(schema) ? schema[0] : schema;
return normalize(entities, Array.isArray(entities) ? [arraySafeSchema] : arraySafeSchema);
}

export function mapMultiEndpointResponses(
action: EntityRequestAction,
catalogueEntity: StratosBaseCatalogueEntity,
requestType: ApiRequestTypes,
multiEndpointResponses: HandledMultiEndpointResponse,
actionDispatcher: (actionToDispatch: Action) => void
): PipelineResult {
const normalizeEntityPipe = normalizeEntityPipeFactory(
catalogueEntity,
// Can this be done outside of the pipe?
// This pipe shouldn't have to worry about the multi entity lists.
patchActionWithForcedConfig(action).schemaKey
);
const endpointErrorHandler = endpointErrorsHandlerFactory(actionDispatcher);
endpointErrorHandler(
action,
Expand All @@ -84,23 +91,28 @@ export function mapMultiEndpointResponses(
errorMessage
};
} else {
const responses = multiEndpointResponses.successes.map(normalizeEntityPipe);
const mapped = responses.map(endpointResponse => {
const entities = getEntities(endpointResponse, action);
const parentEntities = entities[catalogueEntity.entityKey];
return {
response: {
entities,
// If we changed the guid of the entities then make sure this is reflected in the result array.
result: parentEntities ? Object.keys(parentEntities) : endpointResponse.normalizedEntities.result,
},
totalPages: endpointResponse.totalPages,
totalResults: endpointResponse.totalResults,
success: null
};
});
// NormalizedResponse
const response = multiEndpointResponseMergePipe(mapped);
const responses = multiEndpointResponses.successes
.map((responseData: MultiEndpointResponse<any>) => ({
normalizedEntities: getNormalizedEntityData(responseData.entities, action, catalogueEntity),
endpointGuid: responseData.endpointGuid,
totalResults: responseData.totalResults,
totalPages: responseData.totalPages
}))
.map(endpointResponse => {
const entities = getEntities(endpointResponse, action);
const parentEntities = entities[catalogueEntity.entityKey];
return {
response: {
entities,
// If we changed the guid of the entities then make sure this is reflected in the result array.
result: parentEntities ? Object.keys(parentEntities) : endpointResponse.normalizedEntities.result,
},
totalPages: endpointResponse.totalPages,
totalResults: endpointResponse.totalResults,
success: null
};
});
const response = multiEndpointResponseMergePipe(responses);
return {
...response,
success: true,
Expand Down

This file was deleted.

0 comments on commit 7c94858

Please sign in to comment.