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

Limit Sample Collection Date #226

Merged
merged 9 commits into from
Aug 15, 2022

Conversation

SyedAli-789
Copy link
Contributor

limit the collection date that can be selected for a sample.

Changes made to prevent user form submitting blank surveys in cooking oils and surfer questioner.
upper_limit .setDate(upper_limit.getDate());
upper_limit .setMonth(upper_limit.getMonth() + 1);
if(date.getTime() < lower_limit.getTime()) {
alert('Please select a date within the last 10 years.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please wrap this string in gettext shorthand (e.g. {{ _('Please select...') }}

alert('Please select a date within the last 10 years.')
e.preventDefault(e);
} else if (date.getTime() > upper_limit.getTime()) {
alert('Please select a date within the next 30 days.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please wrap this string in gettext shorthand

form_submitted = 1;
}
else {
alert("It looks like you're trying to submit this survey without answering any questions. If you'd prefer not to complete this survey, please return to the homepage.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please wrap this string in gettext shorthand

used gettext shorthand for alert message
used gettext shorthand for alert message
changed you're to you are in the alert text
Added the messages for for survey and sample jinja2 file
Comment on lines 24 to 25
lower_limit .setDate(lower_limit.getDate());
lower_limit .setMonth(lower_limit.getMonth());
Copy link
Collaborator

Choose a reason for hiding this comment

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

These lines are unnecessary

Suggested change
lower_limit .setDate(lower_limit.getDate());
lower_limit .setMonth(lower_limit.getMonth());

Comment on lines 27 to 28
upper_limit .setFullYear( upper_limit.getFullYear() );
upper_limit .setDate(upper_limit.getDate());
Copy link
Collaborator

Choose a reason for hiding this comment

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

These lines are unnecessary

Suggested change
upper_limit .setFullYear( upper_limit.getFullYear() );
upper_limit .setDate(upper_limit.getDate());

upper_limit .setFullYear( upper_limit.getFullYear() );
upper_limit .setDate(upper_limit.getDate());
upper_limit .setMonth(upper_limit.getMonth() + 1);
if(date.getTime() < lower_limit.getTime()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can directly compare date objects with the < and > operators, you just can't check equality directly.

Suggested change
if(date.getTime() < lower_limit.getTime()) {
if(date < lower_limit) {

if(date.getTime() < lower_limit.getTime()) {
alert("{{ _('Please select a date within the last 10 years.') }}")
e.preventDefault(e);
} else if (date.getTime() > upper_limit.getTime()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can directly compare date objects with the < and > operators, you just can't check equality directly.

Suggested change
} else if (date.getTime() > upper_limit.getTime()) {
} else if (date > upper_limit) {

@cassidysymons cassidysymons merged commit b936253 into biocore:master Aug 15, 2022
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.

2 participants