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

RDISCROWD-5852 Calc task expiration by create date #836

Merged
merged 12 commits into from
Apr 18, 2023

Conversation

n00rsy
Copy link

@n00rsy n00rsy commented Apr 5, 2023

Issue number of the reported bug or feature request: RDISCROWD-5852

Describe your changes

Previously
In #830, we limited the task expiration based on the current date. The task expiration limit was n days from the current day. This allowed for the user to indefinitely extend their tasks' expirations, which we don't want.
Now
Task expiration limit is now n days from the create date of the task, putting a hard cap on task expiration.

Testing performed
Tested locally

Additional context
#830

@n00rsy n00rsy requested a review from dchhabda April 5, 2023 13:42
@coveralls
Copy link

coveralls commented Apr 5, 2023

Pull Request Test Coverage Report for Build 4734536223

  • 24 of 24 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 94.111%

Totals Coverage Status
Change from base Build 4564950531: 0.002%
Covered Lines: 16860
Relevant Lines: 17915

💛 - Coveralls

Copy link

@dchhabda dchhabda left a comment

Choose a reason for hiding this comment

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

If user sets expiration beyond max expiration from task create date, will api report 403 along with appropriate message such as Task expiration cannot be set beyond max expiration or it silently sets max expiration from create date and no error reported? If its later, I think NOOP and reporting 403 and appropriate error would be more useful.

@n00rsy
Copy link
Author

n00rsy commented Apr 10, 2023

If user sets expiration beyond max expiration from task create date, will api report 403 along with appropriate message such as Task expiration cannot be set beyond max expiration or it silently sets max expiration from create date and no error reported? If its later, I think NOOP and reporting 403 and appropriate error would be more useful.

Should we implement similar error reporting behavior for importing tasks? the same get_task_expiration function is used in importing and the task api. See: https://github.com/bloomberg/pybossa/blob/main/pybossa/importers/importer.py#L211

@dchhabda
Copy link

Should we implement similar error reporting behavior for importing tasks? the same get_task_expiration function is used in importing and the task api.

Yes, as this error would occur with task create_date null/empty (which is invalid) and system should not have tasks created without setting create date. In case of import, code flow need to ensure that error is handled so that all possible tasks gets imported.

@@ -64,7 +64,6 @@ def set_gold_answers(task, gold_answers):
if encrypted():
url = upload_files_priv(task, task.project_id, gold_answers, TASK_PRIVATE_GOLD_ANSWER_FILE_NAME)['externalUrl']
gold_answers = dict([(TASK_GOLD_ANSWER_URL_KEY, url)])

Choose a reason for hiding this comment

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

I think setting gold task should not be setting task expiration as both operations are independent and hence task expiration not to be modified with setting gold task.

@kbecker42 , @XiChenn , @n00rsy thoughts?

@dchhabda dchhabda force-pushed the RDISCROWD-5852-calc-task-expr-by-create-date branch from 0f74350 to 8faf13a Compare April 17, 2023 14:13
@dchhabda dchhabda requested a review from XiChenn April 17, 2023 15:16
pybossa/util.py Outdated Show resolved Hide resolved
Copy link

@XiChenn XiChenn left a comment

Choose a reason for hiding this comment

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

lgtm

@dchhabda dchhabda merged commit 2f38b2b into main Apr 18, 2023
@dchhabda dchhabda deleted the RDISCROWD-5852-calc-task-expr-by-create-date branch July 10, 2023 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants