Skip to content

Commit

Permalink
Fix reading analysis.active database field (#903)
Browse files Browse the repository at this point in the history
This field is reported by the database as b'\x00' or b'\x01', both of
which bool(...) returns as True. Use parse_sql_bool() instead; and I've
verified that all other BOOLEAN or BIT fields use parse_sql_bool() too.

Add test case exercising the update_analysis() API fixed in PR #899
and checking that active is returned from the database correctly.

* Also fix typo so grepping finds this comment (trivial)
(Verified that changing this XML comment doesn't affect liquibase's
hash for this changeSet.)

* Bump version: 7.4.0 → 7.4.1
  • Loading branch information
jmarshall authored Aug 20, 2024
1 parent 1eb946f commit 209e6e8
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .bumpversion.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 7.4.0
current_version = 7.4.1
commit = True
tag = False
parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>[A-z0-9-]+)
Expand Down
2 changes: 1 addition & 1 deletion api/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from db.python.utils import get_logger

# This tag is automatically updated by bump2version
_VERSION = '7.4.0'
_VERSION = '7.4.1'


logger = get_logger()
Expand Down
2 changes: 1 addition & 1 deletion db/project.xml
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@
<!-- In MariaDB, you can do _conditional_ unique values in a sort of hacky way.
You make a combined key, with one of the fields only NOT NULL when the row should participant in
the check.
IE: if you archive a sequencing group, marking the "nullIfInactitve field to NULL will allow
IE: if you archive a sequencing group, marking the "nullIfInactive" field to NULL will allow
multiple
copies - allowing for us to reuse the same external_ids for multiple sequencing groups. -->
<createTable tableName="sequencing_group_external_id">
Expand Down
2 changes: 1 addition & 1 deletion deploy/python/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.4.0
7.4.1
4 changes: 2 additions & 2 deletions models/models/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from pydantic import BaseModel

from models.base import SMBase, parse_sql_dict
from models.base import SMBase, parse_sql_bool, parse_sql_dict
from models.enums import AnalysisStatus
from models.utils.cohort_id_format import (
cohort_id_format_list,
Expand Down Expand Up @@ -64,7 +64,7 @@ def from_db(**kwargs):
timestamp_completed=timestamp_completed,
project=kwargs.get('project'),
meta=meta,
active=bool(kwargs.get('active')),
active=parse_sql_bool(kwargs.get('active')),
author=kwargs.get('author'),
)

Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
setup(
name=PKG,
# This tag is automatically updated by bump2version
version='7.4.0',
version='7.4.1',
description='Python API for interacting with the Sample API system',
long_description=readme,
long_description_content_type='text/markdown',
Expand Down
35 changes: 35 additions & 0 deletions test/test_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import time
from test.testbase import DbIsolatedTest, run_as_sync

from api.routes.analysis import AnalysisUpdateModel, update_analysis
from db.python.filters import GenericFilter
from db.python.layers.analysis import AnalysisLayer
from db.python.layers.assay import AssayLayer
Expand All @@ -17,6 +18,7 @@
ParticipantUpsertInternal,
SampleUpsertInternal,
SequencingGroupUpsertInternal,
parse_sql_bool,
)


Expand Down Expand Up @@ -323,3 +325,36 @@ async def test_update_analysis(self):
]

self.assertEqual(analyses, expected)

@run_as_sync
async def test_route_update_active(self):
"""
Test that update_analysis(active=False) is effective
"""
analysis_id = await self.al.create_analysis(
AnalysisInternal(
type='cram',
status=AnalysisStatus.COMPLETED,
sequencing_group_ids=[self.genome_sequencing_group_id],
)
)

analyses = await self.connection.connection.fetch_all('SELECT * FROM analysis')
self.assertEqual(1, len(analyses))
self.assertEqual(analysis_id, analyses[0]['id'])
self.assertTrue(parse_sql_bool(analyses[0]['active']))

inactivate = AnalysisUpdateModel(active=False, status=AnalysisStatus.COMPLETED)
await update_analysis(analysis_id, inactivate, self.connection)

analyses = await self.connection.connection.fetch_all('SELECT * FROM analysis')
self.assertEqual(1, len(analyses))
self.assertEqual(analysis_id, analyses[0]['id'])
self.assertFalse(parse_sql_bool(analyses[0]['active']))

analyses = await self.al.query(
AnalysisFilter(project=GenericFilter(eq=self.project_id))
)
self.assertEqual(1, len(analyses))
self.assertEqual(analysis_id, analyses[0].id)
self.assertFalse(analyses[0].active)
2 changes: 1 addition & 1 deletion web/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "metamist",
"version": "7.4.0",
"version": "7.4.1",
"private": true,
"dependencies": {
"@apollo/client": "^3.7.3",
Expand Down

0 comments on commit 209e6e8

Please sign in to comment.