From 08ad618096104908f6c85ab24af28c6d315d3e6a Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Sat, 7 Nov 2026 22:01:08 +0000 Subject: [PATCH 1/2] Lint: no non-null assertions in the GSA form (52 -> 0) A dataset in the store fills in as it loads (its summary, annotations, statistical design), so each reader now says what it needs: - The reducer changes a design through updateDesign and a summary through fromSummary, which leave the state as it is before the dataset has one, as the entity helper already does for a dataset there is not. Annotation columns are made for the datasets that have annotations. - The selectors read the design once, and say "not complete" where it is missing rather than throwing. - The submitted design is built by submittedDesign, tested: a design without its groups or a covariate's column is refused through the path a refused request takes (loadFailure, its reason shown), not sent without them for the service to refuse. - A finished load with no dataset id fails as a summary that could not be had. Lint 1,129 -> 1,077 (no-non-null-assertion 203 -> 151). Co-Authored-By: Claude Opus 5.5 --- lint-baseline.json | 4 +- .../src/lib/model/analysis.model.ts | 4 +- .../lib/services/analysis-submission.spec.ts | 62 +++++++- .../src/lib/services/analysis.service.ts | 68 +++++--- .../dataset-source/dataset-source.selector.ts | 5 +- .../src/lib/state/dataset/dataset.effect.ts | 16 +- .../src/lib/state/dataset/dataset.reducer.ts | 150 +++++++++--------- .../src/lib/state/dataset/dataset.selector.ts | 75 +++++---- 8 files changed, 229 insertions(+), 155 deletions(-) diff --git a/lint-baseline.json b/lint-baseline.json index 776b09e99..f3c96efbf 100644 --- a/lint-baseline.json +++ b/lint-baseline.json @@ -1,5 +1,5 @@ { - "warnings": 1129, + "warnings": 1077, "rules": { "@angular-eslint/no-input-rename": 7, "@angular-eslint/no-output-native": 6, @@ -8,7 +8,7 @@ "@typescript-eslint/no-empty-object-type": 1, "@typescript-eslint/no-explicit-any": 98, "@typescript-eslint/no-namespace": 14, - "@typescript-eslint/no-non-null-assertion": 203, + "@typescript-eslint/no-non-null-assertion": 151, "@typescript-eslint/no-unsafe-argument": 76, "@typescript-eslint/no-unsafe-assignment": 146, "@typescript-eslint/no-unsafe-call": 173, diff --git a/projects/reactome-gsa-form/src/lib/model/analysis.model.ts b/projects/reactome-gsa-form/src/lib/model/analysis.model.ts index d65ea6366..e41a525f6 100644 --- a/projects/reactome-gsa-form/src/lib/model/analysis.model.ts +++ b/projects/reactome-gsa-form/src/lib/model/analysis.model.ts @@ -5,7 +5,7 @@ export namespace Request { parameters: Parameter[]; } - interface Dataset { + export interface Dataset { data: string; design: DataInformation; name: string; @@ -13,7 +13,7 @@ export namespace Request { parameters?: Parameter[]; } - interface DataInformation { + export interface DataInformation { analysisGroup: string[]; comparison: Comparison; samples: string[]; diff --git a/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts b/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts index 94e0af7fe..da782e8cc 100644 --- a/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts +++ b/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts @@ -13,7 +13,7 @@ * anything to do with the decision being made here. */ import { describe, expect, it } from 'vitest'; -import { submittedParameters } from './analysis.service'; +import { submittedDesign, submittedParameters } from './analysis.service'; describe('what a GSA request carries', () => { it('leaves out a field the reader never filled in', () => { @@ -42,3 +42,63 @@ describe('what a GSA request carries', () => { ); }); }); + +describe('the statistical design a GSA request carries', () => { + const dataset = (design: object, columns: [string, string[]][]) => + ({ + summary: { title: 'A dataset' }, + statisticalDesign: { defaultCovariances: new Set(), covariances: [], ...design }, + annotationColumns: new Map(columns), + }) as never; + const columns: [string, string[]][] = [ + ['sample_ids', ['s1', 's2']], + ['condition', ['a', 'b']], + ['batch', ['1', '2']], + ]; + + it('names the columns of a complete design, its chosen covariates with them', () => { + expect( + submittedDesign( + dataset( + { + analysisGroup: 'condition', + comparisonGroup1: 'a', + comparisonGroup2: 'b', + covariances: [ + { name: 'batch', value: true }, + { name: 'unused', value: false }, + ], + }, + columns + ) + ) + ).toEqual({ + analysisGroup: ['a', 'b'], + samples: ['s1', 's2'], + comparison: { group1: 'a', group2: 'b' }, + batch: ['1', '2'], + }); + }); + + it('says a design without its groups is not complete, rather than sending it', () => { + expect(submittedDesign(dataset({ analysisGroup: 'condition' }, columns))).toBe( + 'The statistical design of "A dataset" is not complete' + ); + }); + + it('says so too for a covariate with no column', () => { + expect( + submittedDesign( + dataset( + { + analysisGroup: 'condition', + comparisonGroup1: 'a', + comparisonGroup2: 'b', + covariances: [{ name: 'missing', value: true }], + }, + columns + ) + ) + ).toBe('The statistical design of "A dataset" is not complete'); + }); +}); diff --git a/projects/reactome-gsa-form/src/lib/services/analysis.service.ts b/projects/reactome-gsa-form/src/lib/services/analysis.service.ts index f65b0d65d..82654d53a 100644 --- a/projects/reactome-gsa-form/src/lib/services/analysis.service.ts +++ b/projects/reactome-gsa-form/src/lib/services/analysis.service.ts @@ -27,6 +27,31 @@ export function submittedParameters(parameters: Parameter[] | undefined) { .map((param) => ({ name: param.name, value: String(param.value) })); } +/** + * A dataset's statistical design as the analysis is asked for it: the + * annotation columns its design names, and the two groups compared. The form + * asks for a complete design before it submits (selectStatisticalDesignComplete); + * one that is not is said, rather than sent without its groups for the service + * to refuse. + */ +export function submittedDesign(dataset: Dataset): Request.DataInformation | string { + const design = dataset.statisticalDesign; + const column = (name: string | undefined) => + name === undefined ? undefined : dataset.annotationColumns.get(name); + const analysisGroup = column(design.analysisGroup); + const samples = column('sample_ids'); + const { comparisonGroup1: group1, comparisonGroup2: group2 } = design; + const incomplete = `The statistical design of "${dataset.summary.title}" is not complete`; + if (!analysisGroup || !samples || !group1 || !group2) return incomplete; + const covariates: Record = {}; + for (const covariate of design.covariances.filter((cov) => cov.value)) { + const values = column(covariate.name); + if (!values) return incomplete; + covariates[covariate.name] = values; + } + return { analysisGroup, samples, comparison: { group1, group2 }, ...covariates }; +} + @Injectable({ providedIn: 'root', }) @@ -41,34 +66,27 @@ export class AnalysisService { readonly reportStatusUrl = computed(() => `${this.config().apiRoot}/report_status/`); submitQuery(method: Method, parameters: Parameter[], datasets: Dataset[]): Observable { + const submitted: Request.Dataset[] = []; + for (const dataset of datasets) { + const design = submittedDesign(dataset); + // As a refused request is, its reason where the service puts one: the + // effect answers it with loadFailure, which shows the reason. + if (typeof design === 'string') + return throwError(() => new HttpErrorResponse({ status: 400, error: { detail: design } })); + submitted.push({ + data: dataset.summary.id, + name: dataset.summary.title, + type: dataset.summary.type, + parameters: submittedParameters( + dataset.summary.parameters?.filter((para) => para.scope !== 'common') + ), + design, + }); + } const query: Request.Query = { methodName: method.name || 'Method name', parameters: submittedParameters(parameters), - datasets: datasets.map((dataset: Dataset) => ({ - data: dataset.summary!.id, - name: dataset.summary!.title, - type: dataset.summary!.type, - parameters: submittedParameters( - dataset.summary!.parameters?.filter((para) => para.scope !== 'common') - ), - design: { - analysisGroup: dataset.annotationColumns.get(dataset.statisticalDesign.analysisGroup!)!, - samples: dataset.annotationColumns.get('sample_ids')!, - comparison: { - group1: dataset.statisticalDesign!.comparisonGroup1 as string, - group2: dataset.statisticalDesign!.comparisonGroup2 as string, - }, - ...dataset - .statisticalDesign!.covariances.filter((cov) => cov.value) - .reduce( - (covs, cov) => ({ - ...covs, - [cov.name]: dataset.annotationColumns.get(cov.name!)!, - }), - {} - ), - }, - })), + datasets: submitted, }; return this.http.post(this.submitAnalysisUrl(), query, { responseType: 'text' }); } diff --git a/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.selector.ts b/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.selector.ts index a22f7f586..5e3e848d4 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.selector.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.selector.ts @@ -27,8 +27,9 @@ export const datasetSourceFeature = createFeature({ .filter( (datasetSource) => datasetSource.data_types?.some((type) => method.data_types.includes(type)) || - method.data_types.includes(datasetSource.type!) || - method.data_types.includes(datasetSource.id!) + (datasetSource.type !== undefined && + method.data_types.includes(datasetSource.type)) || + method.data_types.includes(datasetSource.id) ); }), selectDatasetSource: (id: string) => createSelector(selectEntities, (entities) => entities[id]), diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts index 7b489f4f3..c0821f8ca 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts @@ -175,11 +175,12 @@ export class DatasetEffects { this.actions$.pipe( ofType(datasetActions.setLoadStatus), filter(({ loadingStatus }) => loadingStatus.status === 'complete'), + // A finished load names the dataset it made; one that does not cannot be + // summarised, and fails as a summary that could not be had. map(({ loadingStatus, id }) => - datasetActions.getSummary({ - datasetId: loadingStatus.dataset_id!, - id, - }) + loadingStatus.dataset_id + ? datasetActions.getSummary({ datasetId: loadingStatus.dataset_id, id }) + : datasetActions.getSummaryError({ error: 'no dataset id', id }) ) ) ); @@ -205,14 +206,15 @@ export class DatasetEffects { }), map(({ summary, id }) => { setTimeout(() => (this.tour.paused ? this.tour.resume() : null), 1000); - const table: string[][] = !summary.sample_metadata + const metadata = summary.sample_metadata; + const table: string[][] = !metadata ? [[''], ...summary.sample_ids.map((sampleId) => [sampleId])] : [ - ['', ...summary.sample_metadata.map((col) => col.name)], + ['', ...metadata.map((col) => col.name)], ...summary.sample_ids.map((sampleId, i) => [ sampleId, // A missing value stays missing (sent as null), as it always was. - ...summary.sample_metadata!.map((column) => column.values[i] as string), + ...metadata.map((column) => column.values[i] as string), ]), ]; return datasetActions.setAnnotations({ annotations: table, id }); diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts index 116ef33e2..46cff4eff 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts @@ -2,12 +2,42 @@ import { ActionReducer, createReducer, on } from '@ngrx/store'; import { datasetAdapter, DatasetState, initialState, PDataset } from './dataset.state'; import { datasetActions } from './dataset.actions'; import { DataSummary } from '../../model/load-dataset.model'; +import { StatisticalDesign } from '../../model/dataset.model'; import { Subset } from '../../model/utils.model'; import { cp, transpose } from 'reactome-table'; import { EntityHelper, isDefined } from '../../utilities/utils'; const helper = new EntityHelper(datasetAdapter); +/** + * A change to a dataset's statistical design, made once it has one + * (initStatisticalDesign); before that, as for a dataset there is not, the + * state is left as it is. + */ +function updateDesign( + id: number, + state: DatasetState, + change: (design: StatisticalDesign, dataset: PDataset) => Partial +): DatasetState { + const dataset = helper.fetch(id, state); + const design = dataset?.statisticalDesign; + if (!dataset || !design) return state; + return datasetAdapter.updateOne( + { id, changes: { statisticalDesign: { ...design, ...change(design, dataset) } } }, + state + ); +} + +/** A change made from a dataset's summary, once it has one (it is loaded, or uploaded). */ +function fromSummary( + id: number, + state: DatasetState, + change: (summary: DataSummary) => Partial +): DatasetState { + const summary = helper.fetch(id, state)?.summary; + return summary ? datasetAdapter.updateOne({ id, changes: change(summary) }, state) : state; +} + export const datasetReducer: ActionReducer = createReducer( initialState, on(datasetActions.add, (state) => @@ -130,21 +160,25 @@ export const datasetReducer: ActionReducer = createReducer( state.ids .map((id) => state.entities[id]) .filter(isDefined) - .map((dataset) => { - const annotations = dataset.annotations!; + // Each that has its annotations (loaded with its summary). + .flatMap((dataset) => { + const annotations = dataset.annotations; + if (!annotations) return []; const annotationsWithSimpleIds = cp(annotations); const [header] = annotationsWithSimpleIds; if (header) header[0] = 'sample_ids'; - return { - id: dataset.id, - changes: { - annotationColumns: new Map( - transpose(annotationsWithSimpleIds).map((col) => [col[0] ?? '', col.slice(1)]) - ), + return [ + { + id: dataset.id, + changes: { + annotationColumns: new Map( + transpose(annotationsWithSimpleIds).map((col) => [col[0] ?? '', col.slice(1)]) + ), + }, }, - }; + ]; }), state ) @@ -176,8 +210,7 @@ export const datasetReducer: ActionReducer = createReducer( ), on(datasetActions.initStatisticalDesign, (state, { id }) => - helper.update(id, state, (dataset) => { - const summary = dataset.summary!; + fromSummary(id, state, (summary) => { const defaultCovariances = new Set( (summary.default_parameters || []) .find((param) => param.name === 'covariates') @@ -209,102 +242,63 @@ export const datasetReducer: ActionReducer = createReducer( ), on(datasetActions.initCovariances, (state, { id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - covariances: - dataset.annotations?.[0] - ?.slice(1) //slice the first element, which is the empty string - .filter((name) => name !== dataset.statisticalDesign!.analysisGroup) - .map((name) => ({ - name, - value: dataset.statisticalDesign!.defaultCovariances.has(name), - })) || [], - }, + updateDesign(id, state, (design, dataset) => ({ + covariances: + dataset.annotations?.[0] + ?.slice(1) //slice the first element, which is the empty string + .filter((name) => name !== design.analysisGroup) + .map((name) => ({ + name, + value: design.defaultCovariances.has(name), + })) || [], })) ), on(datasetActions.setAnalysisGroup, (state, { group, id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - analysisGroup: group, - comparisonGroup1: - group === dataset.statisticalDesign!.defaultComparisonFactor - ? dataset.statisticalDesign!.defaultGroup1 - : undefined, - comparisonGroup2: - group === dataset.statisticalDesign!.defaultComparisonFactor - ? dataset.statisticalDesign!.defaultGroup2 - : undefined, - }, + updateDesign(id, state, (design) => ({ + analysisGroup: group, + comparisonGroup1: group === design.defaultComparisonFactor ? design.defaultGroup1 : undefined, + comparisonGroup2: group === design.defaultComparisonFactor ? design.defaultGroup2 : undefined, })) ), on(datasetActions.setComparisonGroup1, (state, { group, id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - comparisonGroup1: group, - }, - })) + updateDesign(id, state, () => ({ comparisonGroup1: group })) ), on(datasetActions.setComparisonGroup2, (state, { group, id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - comparisonGroup2: group, - }, - })) + updateDesign(id, state, () => ({ comparisonGroup2: group })) ), on(datasetActions.setCovariateValue, (state, { group, value, id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - covariances: dataset.statisticalDesign!.covariances.map((cov) => - cov.name === group ? { ...cov, value } : cov - ), - }, + updateDesign(id, state, (design) => ({ + covariances: design.covariances.map((cov) => (cov.name === group ? { ...cov, value } : cov)), })) ), on(datasetActions.lockCovariateValue, (state, { group, value, id, locked }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - covariances: dataset.statisticalDesign!.covariances.map((cov) => - cov.name === group ? { ...cov, value, locked } : cov - ), - }, + updateDesign(id, state, (design) => ({ + covariances: design.covariances.map((cov) => + cov.name === group ? { ...cov, value, locked } : cov + ), })) ), on(datasetActions.setCovariatesValue, (state, { value, id }) => - helper.update(id, state, (dataset) => ({ - statisticalDesign: { - ...dataset.statisticalDesign!, - covariances: dataset.statisticalDesign!.covariances.map((cov) => ({ - ...cov, - value: cov.locked ? cov.value : value, - })), - }, + updateDesign(id, state, (design) => ({ + covariances: design.covariances.map((cov) => ({ + ...cov, + value: cov.locked ? cov.value : value, + })), })) ), on(datasetActions.setSummaryParameters, (state, { id, parameters }) => - helper.update(id, state, (dataset) => ({ - summary: { - ...dataset.summary!, - parameters, - }, - })) + fromSummary(id, state, (summary) => ({ summary: { ...summary, parameters } })) ), on(datasetActions.resetSummaryParameters, (state, { id }) => - helper.update(id, state, (dataset) => { - const summary = dataset.summary!; + fromSummary(id, state, (summary) => { return { summary: { ...summary, diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts index a01df2df6..37ce0fac3 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts @@ -30,45 +30,42 @@ export const datasetFeature = createFeature({ selectLoadingComplete: (id: number) => createSelector( selectEntities, - (entities) => !!(entities[id]?.loadingStatus!.status === 'complete') + (entities) => entities[id]?.loadingStatus?.status === 'complete' ), selectSummaryComplete: (id: number) => createSelector(selectEntities, (entities) => !!entities[id]?.summary), selectAnnotationComplete: (id: number) => - createSelector( - selectEntities, - (entities) => - !!( - entities[id]?.annotations && - transpose(entities[id]!.annotations as string[][]) // column by column - .slice(1) // exclude row headers - .map((col) => col.slice(1)) // exclude column headers - .filter( - (col) => - col - .filter((s) => s) // remove empty string - .filter( - (value, i) => col.indexOf(value) === i - ) // distinct values - .length >= 2 // only columns with at least 2 distinct values - ).length > 0 - ) - ), // at least 1 column with at least 2 distinct values + createSelector(selectEntities, (entities) => { + const annotations = entities[id]?.annotations; + return ( + !!annotations && + transpose(annotations) // column by column + .slice(1) // exclude row headers + .map((col) => col.slice(1)) // exclude column headers + .filter( + (col) => + col + .filter((s) => s) // remove empty string + .filter( + (value, i) => col.indexOf(value) === i + ) // distinct values + .length >= 2 // only columns with at least 2 distinct values + ).length > 0 + ); + }), // at least 1 column with at least 2 distinct values selectStatisticalDesignComplete: (id: number) => - createSelector( - selectEntities, - (entities) => - !!( - entities[id]?.statisticalDesign?.analysisGroup && - entities[id]!.statisticalDesign!.comparisonGroup1 && - entities[id]!.statisticalDesign!.comparisonGroup2 && - entities[id]!.statisticalDesign!.comparisonGroup1 !== - entities[id]!.statisticalDesign!.comparisonGroup2 - ) - ), + createSelector(selectEntities, (entities) => { + const design = entities[id]?.statisticalDesign; + return !!( + design?.analysisGroup && + design.comparisonGroup1 && + design.comparisonGroup2 && + design.comparisonGroup1 !== design.comparisonGroup2 + ); + }), selectParametersComplete: (id: number) => createSelector( @@ -104,10 +101,11 @@ export const datasetFeature = createFeature({ selectSampleGroups: (id: number) => createSelector(selectEntities, (entities): SampleGroups => { const dataset = entities[id]; - if (!dataset?.annotations) return {}; + const design = dataset?.statisticalDesign; + if (!dataset?.annotations || !design) return {}; const analysisColumn = transpose(dataset.annotations) - .find((column) => column[0] === dataset.statisticalDesign?.analysisGroup) + .find((column) => column[0] === design.analysisGroup) ?.slice(1); if (!analysisColumn) return {}; @@ -118,17 +116,18 @@ export const datasetFeature = createFeature({ name: column[0] ?? '', values: column.slice(1), })) - .filter((column) => dataset!.statisticalDesign!.analysisGroup !== column.name) // Remove analysis group + .filter((column) => design.analysisGroup !== column.name) // Remove analysis group .filter((column) => { // A missing value counts as one, as it always has. const group1Values = new Set(); const group2Values = new Set(); for (let i = 0; i < analysisColumn.length; i++) { const analysedValue = analysisColumn[i]; - for (const [comparisonGroup, values] of [ - [dataset!.statisticalDesign!.comparisonGroup1, group1Values], - [dataset!.statisticalDesign!.comparisonGroup2, group2Values], - ] as [string, Set][]) { + const groups: [string | undefined, Set][] = [ + [design.comparisonGroup1, group1Values], + [design.comparisonGroup2, group2Values], + ]; + for (const [comparisonGroup, values] of groups) { if (analysedValue === comparisonGroup) { const value = column.values[i]; if (values.has(value)) return false; // remove columns where a "pair" is associated more than once to the same condition From 5388605f85c3b35170371228f595af8eaf4227c6 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 11 Nov 2026 19:31:36 +0000 Subject: [PATCH 2/2] GSA form: name a design column the samples lack; the refusal is a 400 From #896's review. The default analysis group comes from the service, not from the annotation header, so the form can show a design complete whose group names a column the samples lack; submitting it said only that the design was not complete. It now names the column, as it does for a chosen covariate with no column. The refusal is made by refusal(), tested as a 400 with its reason in detail: a 503 would be retried every minute as the service being busy. Both tests were seen failing (a 503; the message removed). Co-Authored-By: Claude Opus 5.5 --- .../lib/services/analysis-submission.spec.ts | 22 +++++++++++++++++-- .../src/lib/services/analysis.service.ts | 20 ++++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts b/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts index da782e8cc..f7ad402af 100644 --- a/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts +++ b/projects/reactome-gsa-form/src/lib/services/analysis-submission.spec.ts @@ -13,7 +13,7 @@ * anything to do with the decision being made here. */ import { describe, expect, it } from 'vitest'; -import { submittedDesign, submittedParameters } from './analysis.service'; +import { refusal, submittedDesign, submittedParameters } from './analysis.service'; describe('what a GSA request carries', () => { it('leaves out a field the reader never filled in', () => { @@ -99,6 +99,24 @@ describe('the statistical design a GSA request carries', () => { columns ) ) - ).toBe('The statistical design of "A dataset" is not complete'); + ).toBe( + 'The statistical design of "A dataset" names "missing", which its samples have no column for' + ); + }); + + it('names an analysis group the samples have no column for', () => { + // The default group comes from the service, not the annotation header. + expect( + submittedDesign( + dataset({ analysisGroup: 'tissue', comparisonGroup1: 'a', comparisonGroup2: 'b' }, columns) + ) + ).toBe( + 'The statistical design of "A dataset" names "tissue", which its samples have no column for' + ); + }); + + it('is refused as a 400 with its reason, never the 503 the form retries', () => { + const refused = refusal('why'); + expect([refused.status, refused.error]).toEqual([400, { detail: 'why' }]); }); }); diff --git a/projects/reactome-gsa-form/src/lib/services/analysis.service.ts b/projects/reactome-gsa-form/src/lib/services/analysis.service.ts index 82654d53a..9576adbd8 100644 --- a/projects/reactome-gsa-form/src/lib/services/analysis.service.ts +++ b/projects/reactome-gsa-form/src/lib/services/analysis.service.ts @@ -42,16 +42,31 @@ export function submittedDesign(dataset: Dataset): Request.DataInformation | str const samples = column('sample_ids'); const { comparisonGroup1: group1, comparisonGroup2: group2 } = design; const incomplete = `The statistical design of "${dataset.summary.title}" is not complete`; + // A name the design holds and the samples have no column for: the default + // analysis group comes from the service, not from the annotation header, so + // the form can show a design complete that names a column the samples lack. + const noColumn = (name: string) => + `The statistical design of "${dataset.summary.title}" names "${name}", which its samples have no column for`; + if (design.analysisGroup && !analysisGroup) return noColumn(design.analysisGroup); if (!analysisGroup || !samples || !group1 || !group2) return incomplete; const covariates: Record = {}; for (const covariate of design.covariances.filter((cov) => cov.value)) { const values = column(covariate.name); - if (!values) return incomplete; + if (!values) return covariate.name ? noColumn(covariate.name) : incomplete; covariates[covariate.name] = values; } return { analysisGroup, samples, comparison: { group1, group2 }, ...covariates }; } +/** + * A design the form will not send, as a refused request: a 400 with its reason + * in `detail`, where extractErrorMessage reads the service's. Never a 503, + * which the effect retries every minute as the service being busy. + */ +export function refusal(reason: string): HttpErrorResponse { + return new HttpErrorResponse({ status: 400, error: { detail: reason } }); +} + @Injectable({ providedIn: 'root', }) @@ -71,8 +86,7 @@ export class AnalysisService { const design = submittedDesign(dataset); // As a refused request is, its reason where the service puts one: the // effect answers it with loadFailure, which shows the reason. - if (typeof design === 'string') - return throwError(() => new HttpErrorResponse({ status: 400, error: { detail: design } })); + if (typeof design === 'string') return throwError(() => refusal(design)); submitted.push({ data: dataset.summary.id, name: dataset.summary.title,