Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addressing Feedback from Production Testing #290

Merged
merged 8 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 76 additions & 24 deletions microsetta_interface/implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from datetime import datetime
import base64
import functools
import re
from string import ascii_lowercase
from microsetta_interface.model_i18n import translate_source, \
translate_sample, translate_survey_template, EN_US_KEY, LANGUAGES, \
Expand Down Expand Up @@ -1092,14 +1093,18 @@ def get_account(*, account_id=None):

# Determine if the source/profile has any action items. This framework
# will evolve over time.
non_human_sources = False
for s in sources:
alerts = 0
if s['source_type'] == Source.SOURCE_TYPE_HUMAN:

need_reconsent_d = check_current_consent(
account_id, s[SOURCE_ID], "data"
)
if need_reconsent_d:
alerts += 1
need_reconsent_d = check_current_consent(
account_id, s[SOURCE_ID], "data"
)
if need_reconsent_d:
alerts += 1
else:
non_human_sources = True

s['alerts'] = alerts

Expand All @@ -1113,7 +1118,8 @@ def get_account(*, account_id=None):
return _render_with_defaults('account_overview.jinja2',
account=account,
sources=sources,
japan_user=japan_user)
japan_user=japan_user,
non_human_sources=non_human_sources)


@prerequisite([ACCT_PREREQS_MET])
Expand Down Expand Up @@ -1396,11 +1402,6 @@ def get_fill_source_survey(*,
ctr = 0
trig_ctr = 0
for field in group['fields']:
if need_reconsent:
# If the user has not agreed to the current consent, we
# disable all of the fields.
field['disabled'] = True

if "triggered_by" in field:
field['label'] = str(ctr) + ascii_lowercase[trig_ctr]\
+ ". " + field['label']
Expand All @@ -1410,10 +1411,22 @@ def get_fill_source_survey(*,
ctr += 1
survey_question_count += 1
field['label'] = str(ctr) + ". " + field['label']
field['label'] = '<span class="survey-skip small-text" '\
+ 'onClick="skipQuestion(this)">'\
+ gettext('SKIP')\
+ '</span>' + field['label']

if need_reconsent:
# If the user has not agreed to the current consent, we
# disable all of the fields.
field['disabled'] = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of paranoia, and the fun you can have in a javascript console, do we assert consent prior to storing in the backend? I think yes but just checking...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# And we remove the onClick action from the Skip/Display
# text, but leave it to reflect their last response
field['label'] = '<span ' \
+ 'class="survey-skip-dis small-text">' \
+ gettext('SKIP') \
+ '</span>' + field['label']
else:
field['label'] = '<span class="survey-skip small-text" '\
+ 'onClick="skipQuestion(this)">'\
+ gettext('SKIP')\
+ '</span>' + field['label']

return _render_with_defaults(
"survey.jinja2",
Expand Down Expand Up @@ -1481,14 +1494,16 @@ def get_fill_vioscreen_remote_sample_survey(*,
account_id=None,
source_id=None,
sample_id=None,
registration_code=None):
registration_code=None,
vio_id=None):
suffix = "vspassthru"
redirect_url = SERVER_CONFIG["endpoint"] + \
_make_source_path(account_id, source_id, suffix=suffix)
params = {
'survey_redirect_url': redirect_url,
'vioscreen_ext_sample_id': sample_id,
'registration_code': registration_code
'registration_code': registration_code,
'vio_id': vio_id
}
has_error, survey_output, _ = ApiRequest.get(
'/accounts/%s/sources/%s/survey_templates/%s' %
Expand Down Expand Up @@ -1721,7 +1736,8 @@ def get_source(*, account_id=None, source_id=None):
if survey['survey_template_type'] == "local":
local_surveys.append(survey)
else:
if survey['survey_template_id'] != VIOSCREEN_ID:
if survey['survey_template_id'] != VIOSCREEN_ID and\
survey['survey_template_id'] != POLYPHENOL_FFQ_ID:
if survey['survey_template_id'] == SPAIN_FFQ_ID and\
spain_user is False:
continue
Expand Down Expand Up @@ -1832,17 +1848,23 @@ def get_kits(*, account_id=None, source_id=None, check_survey_date=False):
for sample in samples_output:
if sample['sample_datetime'] is not None:
dt = datetime.fromisoformat(sample['sample_datetime'])
sample['ts_for_sort'] = dt
# rebase=True - show in user's locale, rebase=False, UTC (I think?)
sample['sample_datetime'] = flask_babel.format_datetime(
dt,
format=None, # Use babel default (short/medium/long/full)
rebase=False)
else:
# We just need a sort value for samples without a collection date
# and we want it to filter to the top when we sort by date desc.
sample['ts_for_sort'] = datetime.fromisoformat("9999-12-31")

is_human = source_output['source_type'] == Source.SOURCE_TYPE_HUMAN

samples = [translate_sample(s) for s in samples_output]

kits = defaultdict(list)
kits_ts = {}
for s in samples:
if s['sample_site'] == '' or s['sample_datetime'] == '':
s['css_class'] = "sample-needs-info"
Expand All @@ -1852,6 +1874,18 @@ def get_kits(*, account_id=None, source_id=None, check_survey_date=False):
s['alert_icon'] = "green_checkmark.svg"

kits[s['kit_id']].append(s)
if s['kit_id'] in kits_ts:
if s['ts_for_sort'] > kits_ts[s['kit_id']]:
kits_ts[s['kit_id']] = s['ts_for_sort']
else:
kits_ts[s['kit_id']] = s['ts_for_sort']

sorted_kits = {}
sorted_kits_ts = dict(
sorted(kits_ts.items(), key=lambda x: x[1], reverse=True)
)
for kit_id in sorted_kits_ts.keys():
sorted_kits[kit_id] = kits[kit_id]

profile_has_samples = _check_if_source_has_samples(account_id, source_id)

Expand Down Expand Up @@ -1879,7 +1913,7 @@ def get_kits(*, account_id=None, source_id=None, check_survey_date=False):
account_id=account_id,
source_id=source_id,
is_human=is_human,
kits=kits,
kits=sorted_kits,
source_name=source_output['source_name'],
fundrazr_url=SERVER_CONFIG["fundrazr_url"],
account_country=account_country,
Expand Down Expand Up @@ -1928,6 +1962,9 @@ def get_nutrition(*, account_id=None, source_id=None, new_ffq_code=None):
return vioscreen_output

profile_has_samples = _check_if_source_has_samples(account_id, source_id)
need_reconsent_data = check_current_consent(
account_id, source_id, "data"
)

return _render_with_defaults(
'nutrition.jinja2',
Expand All @@ -1940,7 +1977,8 @@ def get_nutrition(*, account_id=None, source_id=None, new_ffq_code=None):
nutrition_tab_whitelist=NUTRITION_TAB_WHITELIST,
new_ffq_code=new_ffq_code,
profile_has_samples=profile_has_samples,
has_basic_info=has_basic_info
has_basic_info=has_basic_info,
need_reconsent_data=need_reconsent_data
)


Expand Down Expand Up @@ -2011,9 +2049,7 @@ def get_consents(*, account_id=None, source_id=None):
)
if has_error:
if has_error == 404:
# If historical users who haven't re-consented try to view signed
# consents, we redirect them to the consent page
return render_consent_page(account_id, source_id, "data")
data_consent = None
else:
return data_consent

Expand Down Expand Up @@ -2074,14 +2110,20 @@ def get_consent_view(*, account_id=None, source_id=None, consent_type=None):
False
)

if consent_type == "biospecimen":
consent_type_display = "Biospecimen"
else:
consent_type_display = "Survey"

return _render_with_defaults(
'signed_consent.jinja2',
account_id=account_id,
source_id=source_id,
source_age=source_output['consent']['age_range'],
source_name=source_output['source_name'],
consent=consent_output,
tl=consent_assets
tl=consent_assets,
consent_type_display=consent_type_display
)


Expand Down Expand Up @@ -2480,12 +2522,22 @@ def admin_barcode_search_query_qiita(body):


def get_ajax_check_kit_valid(kit_name):
if re.match(
r"[dD][mM][kK][234689ACDEFHJKMNPRTVWXYacdefhjkmnprtvwxy]{6}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this and not re.match(r"DMK[234689ACDEFHJKMNPRTVWXY]{6}", kit_name.upper())? I think its the same in the end but maybe im missing something. If it is, benefit is easier to read regex imo...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, adjusting.

kit_name
):
kit_name = kit_name.upper()
kit, error, _ = _get_kit(kit_name)
result = True if error is None else error
return flask.jsonify(result)


def get_ajax_list_kit_samples(kit_name):
if re.match(
r"[dD][mM][kK][234689ACDEFHJKMNPRTVWXYacdefhjkmnprtvwxy]{6}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to define the regex once, maybe at global level, eg:

DAKLAPACK_KIT_RE = re.compile(r"...")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusting.

kit_name
):
kit_name = kit_name.upper()
kit, error, code = _get_kit(kit_name)
result = kit if error is None else error
return flask.jsonify(result), code
Expand Down
5 changes: 5 additions & 0 deletions microsetta_interface/routes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,11 @@ paths:
schema:
type: string
nullable: true
- in: query
name: vio_id
schema:
type: string
nullable: true
responses:
'200':
description: Take the vioscreen remote survey
Expand Down
25 changes: 22 additions & 3 deletions microsetta_interface/static/css/minimal_interface.css
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ h2.account-h2 {

h2.profile-card-h2 {
color: var(--tmi-blue);
height: 40px;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
max-width: 90%;
}

h3 {
Expand Down Expand Up @@ -458,7 +463,7 @@ p.tmi-content {

@media (max-width: 575.98px) {
.card-survey {
height: 230px;
height: 280px;
}

.card-survey-external {
Expand Down Expand Up @@ -683,8 +688,8 @@ h1.profile-h1 {
color: var(--tmi-gray-80);
}

@media (max-width: 575.98px) {
.survey-info-row {
@media (max-width: 359.98px) {
.survey-info-row > .text-end {
display: none;
}
}
Expand Down Expand Up @@ -894,6 +899,16 @@ li.active-profile {
text-decoration: underline;
}

.survey-skip-dis {
float: right;
margin-right: 10px;
color: var(--tmi-gray-80) !important;
font-weight: 400 !important;
font-size: 12px !important;
line-height: 20px !important;
text-decoration: none;
}

.skip-text {
color: var(--tmi-gray-80) !important;
font-weight: 400 !important;
Expand Down Expand Up @@ -1267,6 +1282,10 @@ input.barcode-checkbox[type=checkbox]:checked+label {
margin-left: auto;
}

.barcode-col > .btn {
display: inline;
}

.barcode-text {
color: var(--tmi-blue);
font-weight: 700;
Expand Down
30 changes: 29 additions & 1 deletion microsetta_interface/templates/account_overview.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
<div class="container default-container">
{% if sources|length > 0 %}
<h2 class="account-h2">{{ _('Active Profiles') }}</h2>
{% if non_human_sources %}
<p class="tmi-tooltip">{{ _('Please note: animal and environmental profiles are currently unavailable.') }}</p>
{% endif %}
<div class="row mb-5">
{% for source in sources %}
<div class="col-md-4 mb-4">
Expand All @@ -66,13 +69,31 @@
</div>
</div>
{% else %}
{% if source.source_type == "animal" %}
<div class="card card-profile">
<img src="/static/img/source_animal.png" class="mx-auto card-profile-icon" width="70px">
<h2 class="profile-card-h2 mx-auto mt-2 mb-4" title="{{ source.source_name|e}}">{{ source.source_name|e}}</h2>
<a class="btn btn-blue-gradient mx-auto" style="width: fit-content; pointer-events: none;" href="#" disabled>
{{ _('Unavailable') }}
</a>
</div>
{% elif source.source_type == "environmental" %}
<div class="card card-profile">
<img src="/static/img/source_environmental.png" class="mx-auto card-profile-icon" width="70px">
<h2 class="profile-card-h2 mx-auto mt-2 mb-4" title="{{ source.source_name|e}}">{{ source.source_name|e}}</h2>
<a class="btn btn-blue-gradient mx-auto" style="width: fit-content; pointer-events: none;" href="#" disabled>
{{ _('Unavailable') }}
</a>
</div>
{% else %}
<div class="card card-profile">
<img src="/static/img/profile_card_icon.png" class="mx-auto card-profile-icon">
<h2 class="profile-card-h2 mx-auto mt-2 mb-4">{{ source.source_name|e}}</h2>
<h2 class="profile-card-h2 mx-auto mt-2 mb-4" title="{{ source.source_name|e}}">{{ source.source_name|e}}</h2>
<a class="btn btn-blue-gradient mx-auto" style="width: fit-content;" href="/accounts/{{account.account_id}}/sources/{{ source.source_id|e }}">
{{ _('Go to My Profile') }}
</a>
</div>
{% endif %}
{% endif %}
</div>
{% endfor %}
Expand Down Expand Up @@ -106,6 +127,13 @@
<p class="disabled-source-types"><strong>{{ _('Coming Soon') }}</strong></p>
</div>
</div>
{% if admin_mode %}
<div class="row mt-3">
<div class="col-12">
<center><h2><a href="/accounts/{{ account.account_id }}/details">{{ _('Account Details') }}</a></h2></center>
</div>
</div>
{% endif %}
</div>


Expand Down
Loading
Loading