Skip to content

[FIX] increment analysis order on POST instead of always 1#1617

Open
lobennett wants to merge 1 commit into
neurostuff:masterfrom
lobennett:fix/analysis-order-post
Open

[FIX] increment analysis order on POST instead of always 1#1617
lobennett wants to merge 1 commit into
neurostuff:masterfrom
lobennett:fix/analysis-order-post

Conversation

@lobennett

Copy link
Copy Markdown
Contributor

Summary

Fixes #1580. Creating an analysis via POST /api/analyses/ without an explicit order always assigned order = 1, so analyses never received a sequential order within their study.

Root cause

AnalysisSchema.load_values (a @pre_load hook) auto-assigns the order by querying the current max for the study:

if data.get("study_id") is not None:
    max_order = db.session.query(func.max(Analysis.order)).filter_by(study_id=data["study_id"]).scalar()
    data["order"] = 1 if max_order is None else max_order + 1
else:
    data["order"] = 1

The field is declared study_id = fields.String(data_key="study"). @pre_load runs on the raw payload, before marshmallow maps the data_key, so the study id is present under the key study, not study_id. data.get("study_id") was therefore always None, the else branch always ran, and every new analysis got order = 1.

Fix

Resolve the id from the study data_key (keeping the study_id fallback and a nested-object guard), mirroring the pattern PointSchema.process_values already uses for the identical case:

study_id = data.get("study_id") or (
    data.get("study") if isinstance(data.get("study"), str) else None
)
if study_id:
    max_order = db.session.query(func.max(Analysis.order)).filter_by(study_id=study_id).scalar()
    data["order"] = 1 if max_order is None else max_order + 1
else:
    data["order"] = 1

AnalysisSchema is the only change; PointSchema already handled this correctly.

Tests

The existing test_post_analysis_without_order only asserted order is not None, which passed even with the bug (order was 1, not None) — that gap let the bug through. Adds test_post_analyses_without_order_increments_within_study: creates a fresh study and POSTs two order-less analyses, asserting orders 1 then 2. Fails on the old code (both 1), passes with the fix.

python -m pytest neurostore/tests/api/test_analyses.py -k order
# 3 passed

AnalysisSchema.load_values read the study id from data['study_id'], but
the pre_load payload carries it under the 'study' data_key, so the
auto-order branch never ran and every new analysis got order 1. Resolve
the id from 'study' (mirroring PointSchema), keeping the study_id
fallback and a nested-object guard. Closes neurostuff#1580.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

creating new analyses via analyses POST endpoint gives it order of 1

1 participant