Move FFQ's from sample to source #459

SyedAli-789 · 2022-09-19T19:09:49Z

No description provided.

microsetta_private_api/repo/survey_template_repo.py

added parameter to the get the recent timestamp records

microsetta_private_api/repo/survey_template_repo.py

cassidysymons · 2022-10-27T22:16:47Z

microsetta_private_api/repo/survey_template_repo.py

@@ -540,8 +538,7 @@ def get_vioscreen_sample_to_user(self):
                           WHERE vio_id IS NOT NULL""")
            return {r[0]: r[1] for r in cur.fetchall()}

-    def create_vioscreen_id(self, account_id, source_id,
-                            vioscreen_ext_sample_id):
+    def create_vioscreen_id(self, account_id, source_id):


We need to adjust the logic on this function, since a given source can have more than one vioscreen ID associated. Since users will unlock access using a registration code, go ahead and add activation_code as a column to the vioscreen_registry table (column type = varchar). Then, you need to pass the registration_code into get_vioscreen_id_if_exists. If it doesn't, create the new record in vioscreen_registry, including the registration_code.

cassidysymons · 2022-10-27T22:20:15Z

microsetta_private_api/api/_vioscreen.py

    VioscreenDietaryScoreRepo, VioscreenSupplementsRepo,
    VioscreenFoodComponentsRepo, VioscreenEatingPatternsRepo,
    VioscreenMPedsRepo, VioscreenFoodConsumptionRepo
 )


-def _get_session_by_account_details(account_id, source_id, sample_id):
+def _get_session_by_account_details(account_id, source_id):


Please update all of the functions in _vioscreen.py that call get_vioscreen_id_if_exists to include the new optional parameters (both in how they call that function and their own function definition) so we can update the full stack of functions to work with the intended logic. Also, please update the parameters in the YAML file accordingly.

Given that a source can have multiple sessions, will this function now return an iterable? If so then the functions which call it most likely all need to be updated to handle multiple sessions.

No, it should be updated to to use the switching logic between sample_id, registration_code, timestamp, or default to most recent.

microsetta_private_api/repo/vioscreen_repo.py

microsetta_private_api/api/_vioscreen.py

microsetta_private_api/api/microsetta_private_api.yaml

wasade

Added a few comments

@cassidysymons, is there going to be a need to migrate existing data in anyway?

wasade · 2022-10-28T15:58:14Z

microsetta_private_api/api/_vioscreen.py

    VioscreenDietaryScoreRepo, VioscreenSupplementsRepo,
    VioscreenFoodComponentsRepo, VioscreenEatingPatternsRepo,
    VioscreenMPedsRepo, VioscreenFoodConsumptionRepo
 )


-def _get_session_by_account_details(account_id, source_id, sample_id):
+def _get_session_by_account_details(account_id, source_id):


Given that a source can have multiple sessions, will this function now return an iterable? If so then the functions which call it most likely all need to be updated to handle multiple sessions.

wasade · 2022-10-28T15:59:52Z

microsetta_private_api/api/_vioscreen.py

+
+
+def get_vioscreens(account_id, source_id):
+    """Obtain a vioscreen ID if it exists"""


I believe this should note that multiple vioscreen IDs may be returned

Yes. @SyedAli-789 please clarify the comment for this function.

microsetta_private_api/api/microsetta_private_api.yaml

wasade · 2022-10-28T16:01:12Z

microsetta_private_api/api/microsetta_private_api.yaml

+            application/json:
+              schema:
+                type:
+                  object


I believe this is inaccurate, isn't the return an array of object?

wasade · 2022-10-28T16:33:13Z

microsetta_private_api/repo/vioscreen_repo.py

+            if rows is None or len(rows) == 0:
+                return None
+            else:
+                return rows


Can this return instances of the VioscreenSession model?

cassidysymons · 2022-10-28T16:37:28Z

Added a few comments

@cassidysymons, is there going to be a need to migrate existing data in anyway?

@wasade Not that I'm aware of. We're leaving the sample_id column in place and providing a way to search for Vioscreen sessions for historical records using that column.

wasade · 2022-10-28T16:46:20Z

@cassidysymons, thanks. We currently pull out sample <-> vioscreen ID associations to construct FFQ metadata suitable for analysis (see #461) -- it's going to be important that that method remain correct in light of the changes here.

wasade · 2022-10-28T16:46:38Z

Ah sorry, clicked the wrong button

microsetta_private_api/api/_survey.py

cassidysymons · 2022-10-28T21:18:35Z

microsetta_private_api/api/_vioscreen.py

    VioscreenDietaryScoreRepo, VioscreenSupplementsRepo,
    VioscreenFoodComponentsRepo, VioscreenEatingPatternsRepo,
    VioscreenMPedsRepo, VioscreenFoodConsumptionRepo
 )


-def _get_session_by_account_details(account_id, source_id, sample_id):
+def _get_session_by_account_details(account_id, source_id):


No, it should be updated to to use the switching logic between sample_id, registration_code, timestamp, or default to most recent.

microsetta_private_api/api/_vioscreen.py

cassidysymons · 2022-10-28T21:20:34Z

microsetta_private_api/api/_vioscreen.py

+
+
+def get_vioscreens(account_id, source_id):
+    """Obtain a vioscreen ID if it exists"""


Yes. @SyedAli-789 please clarify the comment for this function.

microsetta_private_api/api/microsetta_private_api.yaml

cassidysymons · 2022-11-15T21:52:42Z

microsetta_private_api/api/tests/test_api.py

+        if now.month > 10:
+            delta = relativedelta(month=1, year=now.year+1)
+        else:
+            delta = relativedelta(month=now.month+2)


Please back out this change, the code as is - delta = relativedelta(months=now.month+2) - is a more concise approach.

reverted the changes

wasade · 2022-11-16T00:16:37Z

microsetta_private_api/repo/survey_template_repo.py

+                            "GROUP BY vioscreen_registry.vio_id, "
+                            "ag_login_surveys.creation_time "
+                            "ORDER BY ag_login_surveys.creation_time DESC ",
+                            (account_id, source_id))
            rows = cur.fetchall()


Should this be a fetchone?

wasade · 2022-11-16T00:18:44Z

microsetta_private_api/repo/survey_template_repo.py

+                            "AND vioscreen_registry.source_id = %s "
+                            "AND deleted=false "
+                            "AND ag_login_surveys.creation_time >= %s"
+                            "GROUP BY vioscreen_registry.vio_id, "


Since we're returning the most recent survey below, is the GROUP BY needed?

wasade · 2022-11-16T00:19:23Z

microsetta_private_api/repo/survey_template_repo.py

+                            "WHERE vioscreen_registry.account_id = %s "
+                            "AND vioscreen_registry.source_id = %s "
+                            "AND deleted=false "
+                            "GROUP BY vioscreen_registry.vio_id, "


wasade · 2022-11-16T00:20:25Z

microsetta_private_api/repo/vioscreen_repo.py

@@ -1656,3 +1656,24 @@ def _get_vioscreen_status(self, survey_id):
            if row is None:
                return None
            return row[0]
+
+    def get_vioscreen_sessions(self, account_id, source_id):


Can a unit test for this method be added?

@SyedAli-789 Please add unit tests for this function to repo/test/test_vioscreen.py. There should be one test that reflects the result of no rows being found and one test that reflects the result of one or more results being found.

wasade · 2022-11-16T00:22:02Z

microsetta_private_api/repo/vioscreen_repo.py

+            cur.execute("SELECT * FROM vioscreen_sessions WHERE"
+                        "username IN"
+                        "(SELECT vio_id FROM vioscreen_registry WHERE "
+                        "account_id=%s AND "
+                        "source_id=%s)",
+                        (account_id, source_id))


I think this is the same as

SELECT vs.* FROM vioscreen_sessions vs JOIN vioscreen_registry vr ON vs.username = vr.vio_id WHERE account_id=%s AND source_id=%s

SyedAli-789 added 6 commits September 19, 2022 12:08

Move FFQ's from sample to source

f21eaf9

Move FFQ's from sample to source

6ac01df

test fix

8c08b70

test fix

4c8b06d

test integration file changes

7ea64f7

merge conflict resolved

50588ff

cassidysymons requested changes Oct 14, 2022

View reviewed changes

microsetta_private_api/repo/survey_template_repo.py Outdated Show resolved Hide resolved

SyedAli-789 added 4 commits October 17, 2022 11:01

PR comment changes

8ce863c

lint fix

8c3e68d

lint fix

8213605

lint fix

2ec55e6

cassidysymons requested changes Oct 18, 2022

View reviewed changes

microsetta_private_api/repo/survey_template_repo.py Outdated Show resolved Hide resolved

SyedAli-789 and others added 7 commits October 25, 2022 12:57

PR comment changes

908bf30

lint fix

55a37a3

added parameter

626c901

added parameter to the get the recent timestamp records

lint fix

bed5f7a

lint fix

d5acabc

lint fix

e1319bc

vioscreen session implementation

496c48c

cassidysymons requested changes Oct 27, 2022

View reviewed changes

wasade reviewed Oct 28, 2022

View reviewed changes

wasade closed this Oct 28, 2022

wasade reopened this Oct 28, 2022

SyedAli-789 added 2 commits October 28, 2022 13:09

PR comments addresses

0c714a9

PR comment changes

a5ecb04

cassidysymons requested changes Oct 28, 2022

View reviewed changes

adressed PR comments

fe1f8af

SyedAli-789 added 8 commits November 2, 2022 14:08

chadged params to optional

bffb901

test fix

e4dc6a1

test error fix

4ace98a

sample-vioscreen changes

b2ca0e7

merge conflict resolved

97eb2fe

.

75cc25a

test fix changes

02bfdaf

reverting the changes

27f131b

cassidysymons requested changes Nov 15, 2022

View reviewed changes

SyedAli-789 added 2 commits November 15, 2022 14:17

reverting as per review comments

9c1d3f3

.

827c2a1

wasade reviewed Nov 16, 2022

View reviewed changes

addressed reviw comments

87d4f25

cassidysymons mentioned this pull request Nov 28, 2022

Perk Fulfillment #491

Merged

SyedAli-789 added 11 commits November 30, 2022 13:03

testcase added

ecdfeb7

conflict resolved

8c46eee

unit test updated

5147c5f

test fix

3f8aaaa

test fix

bcd996e

test fix

5cde512

test fix

1bcae76

unite test replaced

ec5e9f8

test fix

d54fc81

test fix

7f69b45

syntax fix

a33654c

cassidysymons merged commit bd04cf1 into biocore:master-overhaul Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move FFQ's from sample to source #459

Move FFQ's from sample to source #459

SyedAli-789 commented Sep 19, 2022

cassidysymons Oct 27, 2022

cassidysymons Oct 27, 2022

wasade Oct 28, 2022

cassidysymons Oct 28, 2022

wasade left a comment

wasade Oct 28, 2022

wasade Oct 28, 2022

cassidysymons Oct 28, 2022

wasade Oct 28, 2022

wasade Oct 28, 2022

cassidysymons commented Oct 28, 2022

wasade commented Oct 28, 2022

wasade commented Oct 28, 2022

cassidysymons Oct 28, 2022

cassidysymons Oct 28, 2022

cassidysymons Nov 15, 2022

SyedAli-789 Nov 15, 2022

wasade Nov 16, 2022

wasade Nov 16, 2022 •

edited

Loading

wasade Nov 16, 2022

wasade Nov 16, 2022

cassidysymons Nov 30, 2022

wasade Nov 16, 2022



		def get_vioscreens(account_id, source_id):
		"""Obtain a vioscreen ID if it exists"""

Move FFQ's from sample to source #459

Move FFQ's from sample to source #459

Conversation

SyedAli-789 commented Sep 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wasade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cassidysymons commented Oct 28, 2022

wasade commented Oct 28, 2022

wasade commented Oct 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wasade Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wasade Nov 16, 2022 •

edited

Loading