From f883eb2bbe1e477203ca90f818cf508073aaa4ba Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Thu, 4 Mar 2021 10:42:13 -0500 Subject: [PATCH 1/5] We now include field.id in the ApiError message for hidden and default validation. (The field must have a default value or value expression) While working on this ticket I found a logic error in the test. I expected both default_value and value_expression. It turns out the default_value is always there, it's just null if we don't set it. Modified the existing test for hidden and required, and added tests to make sure my passing logic is now correct. --- crc/services/workflow_service.py | 5 +- .../hidden_required_field.bpmn | 6 +- .../hidden_required_field_pass.bpmn | 93 ++++++++++++++++++ ...hidden_required_field_pass_expression.bpmn | 96 +++++++++++++++++++ .../test_workflow_hidden_required_field.py | 11 +++ 5 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn create mode 100644 tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index acceee10..3287aa7e 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -156,9 +156,10 @@ class WorkflowService(object): # If a field is hidden and required, it must have a default value or value_expression if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION) and field.has_validation(Task.FIELD_CONSTRAINT_REQUIRED): - if not field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) or not (hasattr(field, 'default_value')): + if not field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) and \ + (not (hasattr(field, 'default_value')) or field.default_value is None): raise ApiError(code='hidden and required field missing default', - message='Fields that are required but can be hidden must have either a default value or a value_expression', + message=f'Field "{field.id}" is required but can be hidden. It must have either a default value or a value_expression', task_id='task.id', task_name=task.get_name()) diff --git a/tests/data/hidden_required_field/hidden_required_field.bpmn b/tests/data/hidden_required_field/hidden_required_field.bpmn index 5e4ace06..6c1eafcd 100644 --- a/tests/data/hidden_required_field/hidden_required_field.bpmn +++ b/tests/data/hidden_required_field/hidden_required_field.bpmn @@ -1,5 +1,5 @@ - + Flow_0zt7wv5 @@ -32,7 +32,9 @@ if not 'hide_yes_no' in globals(): - <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> +<div><span>Color is {{ color }}</span></div> + Flow_1qkjkbh Flow_1udbzd6 diff --git a/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn b/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn new file mode 100644 index 00000000..cdf75fa7 --- /dev/null +++ b/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn @@ -0,0 +1,93 @@ + + + + + Flow_0zt7wv5 + + + + + + + + + + + + + + + + Flow_0fb4w15 + Flow_0c2rym0 + + + + Flow_0cm6imh + Flow_0fb4w15 + if not 'require_yes_no' in globals(): + require_yes_no = True +if not 'hide_yes_no' in globals(): + hide_yes_no = True + + + + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + + Flow_0c2rym0 + Flow_1udbzd6 + + + Flow_1udbzd6 + + + + <H1>Hello</H1> + Flow_0zt7wv5 + Flow_0cm6imh + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn b/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn new file mode 100644 index 00000000..52140d1c --- /dev/null +++ b/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn @@ -0,0 +1,96 @@ + + + + + Flow_0zt7wv5 + + + + + + + + + + + + + + + + + Flow_0fb4w15 + Flow_0c2rym0 + + + + Flow_0cm6imh + Flow_0fb4w15 + if not 'require_yes_no' in globals(): + require_yes_no = True +if not 'hide_yes_no' in globals(): + hide_yes_no = True +if not 'value_expression_value' in globals(): + value_expression_value = 'World' + + + + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + + Flow_0c2rym0 + Flow_1udbzd6 + + + Flow_1udbzd6 + + + + <H1>Hello</H1> + Flow_0zt7wv5 + Flow_0cm6imh + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/workflow/test_workflow_hidden_required_field.py b/tests/workflow/test_workflow_hidden_required_field.py index 9c635992..b77dbb24 100644 --- a/tests/workflow/test_workflow_hidden_required_field.py +++ b/tests/workflow/test_workflow_hidden_required_field.py @@ -14,6 +14,17 @@ class TestWorkflowHiddenRequiredField(BaseTest): self.assertEqual(json_data[0]['code'], 'hidden and required field missing default') self.assertIn('task_id', json_data[0]) self.assertIn('task_name', json_data[0]) + self.assertIn('Field "name" is required but can be hidden', json_data[0]['message']) + + def test_require_default_pass(self): + spec_model = self.load_test_spec('hidden_required_field_pass') + rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers()) + self.assertEqual(0, len(rv.json)) + + def test_require_default_pass_expression(self): + spec_model = self.load_test_spec('hidden_required_field_pass_expression') + rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers()) + self.assertEqual(0, len(rv.json)) def test_default_used(self): # If a field is hidden and required, make sure we use the default value From 291216e08187957cffe7867814cd98bc4ec88778 Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 8 Mar 2021 14:00:03 -0500 Subject: [PATCH 2/5] Fixing a bug that prevented proper evaluation of a enum field where the default value was null or empty. --- crc/__init__.py | 2 +- crc/api/tools.py | 13 +++--- crc/services/workflow_processor.py | 41 ++----------------- crc/services/workflow_service.py | 9 ++-- tests/test_tools_api.py | 12 ++++++ .../test_workflow_value_expression.py | 6 ++- 6 files changed, 34 insertions(+), 49 deletions(-) diff --git a/crc/__init__.py b/crc/__init__.py index 6fd1ebbd..bf434fd1 100644 --- a/crc/__init__.py +++ b/crc/__init__.py @@ -63,7 +63,7 @@ if app.config['SENTRY_ENVIRONMENT']: # Connexion Error handling def render_errors(exception): from crc.api.common import ApiError, ApiErrorSchema - error = ApiError(code=exception.title, message=exception.details, status_code=exception.status) + error = ApiError(code=exception.title, message=exception.detail, status_code=exception.status) return Response(ApiErrorSchema().dump(error), status=401, mimetype="application/json") diff --git a/crc/api/tools.py b/crc/api/tools.py index d3325540..e2640d6c 100644 --- a/crc/api/tools.py +++ b/crc/api/tools.py @@ -11,7 +11,7 @@ from crc.scripts.complete_template import CompleteTemplate from crc.scripts.script import Script import crc.scripts from crc.services.mails import send_test_email -from crc.services.workflow_processor import WorkflowProcessor +from crc.services.workflow_processor import WorkflowProcessor, CustomBpmnScriptEngine def render_markdown(data, template): @@ -76,10 +76,11 @@ def evaluate_python_expression(body): front end application needs to do real-time processing on task data. If for instance there is a hide expression that is based on a previous value in the same form.""" try: - # fixme: The script engine should be pulled from Workflow Processor, - # but the one it returns overwrites the evaluate expression making it uncallable. - script_engine = PythonScriptEngine() - result = script_engine.evaluate(body['expression'], **body['data']) + script_engine = CustomBpmnScriptEngine() + result = script_engine.eval(body['expression'], body['data']) return {"result": result} except Exception as e: - raise ApiError("expression_error", str(e)) + raise ApiError("expression_error", f"Failed to evaluate the expression '%s'. %s" % + (body['expression'], str(e)), + task_data = body["data"]) + diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 1c66c91a..0364c36a 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -68,43 +68,6 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): f'something you are referencing does not exist:' f' {script}, {e}') - # else: - # self.run_predefined_script(task, script[2:], data) # strip off the first two characters. - - # def run_predefined_script(self, task: SpiffTask, script, data): - # commands = shlex.split(script) - # path_and_command = commands[0].rsplit(".", 1) - # if len(path_and_command) == 1: - # module_name = "crc.scripts." + self.camel_to_snake(path_and_command[0]) - # class_name = path_and_command[0] - # else: - # module_name = "crc.scripts." + path_and_command[0] + "." + self.camel_to_snake(path_and_command[1]) - # class_name = path_and_command[1] - # try: - # mod = __import__(module_name, fromlist=[class_name]) - # klass = getattr(mod, class_name) - # study_id = task.workflow.data[WorkflowProcessor.STUDY_ID_KEY] - # if WorkflowProcessor.WORKFLOW_ID_KEY in task.workflow.data: - # workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] - # else: - # workflow_id = None - # - # if not isinstance(klass(), Script): - # raise ApiError.from_task("invalid_script", - # "This is an internal error. The script '%s:%s' you called " % - # (module_name, class_name) + - # "does not properly implement the CRC Script class.", - # task=task) - # if task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: - # """If this is running a validation, and not a normal process, then we want to - # mimic running the script, but not make any external calls or database changes.""" - # klass().do_task_validate_only(task, study_id, workflow_id, *commands[1:]) - # else: - # klass().do_task(task, study_id, workflow_id, *commands[1:]) - # except ModuleNotFoundError: - # raise ApiError.from_task("invalid_script", - # "Unable to locate Script: '%s:%s'" % (module_name, class_name), - # task=task) def evaluate_expression(self, task, expression): """ @@ -130,6 +93,10 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): "Error evaluating expression " "'%s', %s" % (expression, str(e))) + def eval(self, exp, data): + return super()._eval(exp, {}, **data) + + @staticmethod def camel_to_snake(camel): diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index acceee10..5aa9635f 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -262,12 +262,13 @@ class WorkflowService(object): # If no default exists, return None # Note: if default is False, we don't want to execute this code - if default is None: - return None + if default is None or (isinstance(default, str) and default.strip() == ''): + if field.type == "enum" or field.type == "autocomplete": + return {'value': None, 'label': None} + else: + return None if field.type == "enum" and not has_lookup: - if isinstance(default, str) and default.strip() == '': - return default_option = next((obj for obj in field.options if obj.id == default), None) if not default_option: raise ApiError.from_task("invalid_default", "You specified a default value that does not exist in " diff --git a/tests/test_tools_api.py b/tests/test_tools_api.py index 67cc963d..f0c2e3ca 100644 --- a/tests/test_tools_api.py +++ b/tests/test_tools_api.py @@ -48,6 +48,7 @@ class TestStudyApi(BaseTest): response = json.loads(rv.get_data(as_text=True)) self.assertEqual(True, response['result']) + def test_eval_expression_with_strings(self): """Assures we can use python to process a value expression from the front end""" rv = self.app.put('/v1.0/eval', @@ -59,3 +60,14 @@ class TestStudyApi(BaseTest): self.assert_success(rv) response = json.loads(rv.get_data(as_text=True)) self.assertEqual('Hello, Trillian Astra!!!', response['result']) + + def test_eval_to_boolean_expression_with_dot_notation(self): + """Assures we can use python to process a value expression from the front end""" + rv = self.app.put('/v1.0/eval', + data='{"expression": "test.value", "data": {"test":{"value": true}}}', + follow_redirects=True, + content_type='application/json', + headers=self.logged_in_headers()) + self.assert_success(rv) + response = json.loads(rv.get_data(as_text=True)) + self.assertEqual(True, response['result']) \ No newline at end of file diff --git a/tests/workflow/test_workflow_value_expression.py b/tests/workflow/test_workflow_value_expression.py index e059dfef..d527c694 100644 --- a/tests/workflow/test_workflow_value_expression.py +++ b/tests/workflow/test_workflow_value_expression.py @@ -14,7 +14,11 @@ class TestValueExpression(BaseTest): workflow_api = self.get_workflow_api(workflow) second_task = workflow_api.next_task self.assertEqual('', second_task.data['value_expression_value']) - self.assertNotIn('color', second_task.data) + # self.assertNotIn('color', second_task.data) + self.assertIn('color', second_task.data) + self.assertIsNone(second_task.data['color']['value']) + + def test_value_expression_with_default(self): From 576aebab19ebd1f7e18397eda7e0638e4d652c01 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 9 Mar 2021 11:52:55 -0500 Subject: [PATCH 3/5] Sometimes a workflow can be completely broken and unloadable. For instance, it starts off with a engine step / script task that will not execute. So the failure happens the second it is processed. When calling reset on a workflow, we should catch the failure, and reset the workflow, so we aren't trapped in an unrecoverable state. To do so, I changed the WorkflowProcessor's reset method to be static, it will try to instantiate the current workflow and execute a cancel_notify event, but if that fails, it will continue and always return a new workflow processor using the latest spec. --- crc/api/workflow.py | 2 +- crc/services/workflow_processor.py | 15 ++++++++++++--- crc/services/workflow_service.py | 3 +-- tests/test_lookup_service.py | 6 ++---- tests/test_tasks_api.py | 21 +++++++++++++++++++++ tests/workflow/test_workflow_processor.py | 2 +- 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/crc/api/workflow.py b/crc/api/workflow.py index 5c510877..747d2cbe 100644 --- a/crc/api/workflow.py +++ b/crc/api/workflow.py @@ -120,7 +120,7 @@ def restart_workflow(workflow_id, clear_data=False): """Restart a workflow with the latest spec. Clear data allows user to restart the workflow without previous data.""" workflow_model: WorkflowModel = session.query(WorkflowModel).filter_by(id=workflow_id).first() - WorkflowProcessor(workflow_model).reset(workflow_model, clear_data=clear_data) + WorkflowProcessor.reset(workflow_model, clear_data=clear_data) return get_workflow(workflow_model.id) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 0364c36a..56eb1e89 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -173,10 +173,19 @@ class WorkflowProcessor(object): else: self.is_latest_spec = False - def reset(self, workflow_model, clear_data=False): + @staticmethod + def reset(workflow_model, clear_data=False): print('WorkflowProcessor: reset: ') - self.cancel_notify() + # Try to execute a cancel notify + try: + wp = WorkflowProcessor(workflow_model) + wp.cancel_notify() # The executes a notification to all endpoints that + except Exception as e: + app.logger.error(f"Unable to send a cancel notify for workflow %s during a reset." + f" Continuing with the reset anyway so we don't get in an unresolvable" + f" state. An %s error occured with the following information: %s" % + (workflow_model.id, e.__class__.__name__, str(e))) workflow_model.bpmn_workflow_json = None if clear_data: # Clear form_data from task_events @@ -186,7 +195,7 @@ class WorkflowProcessor(object): task_event.form_data = {} session.add(task_event) session.commit() - return self.__init__(workflow_model) + return WorkflowProcessor(workflow_model) def __get_bpmn_workflow(self, workflow_model: WorkflowModel, spec: WorkflowSpec, validate_only=False): if workflow_model.bpmn_workflow_json: diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 2a8209b3..60ec8898 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -731,5 +731,4 @@ class WorkflowService(object): workflows = db.session.query(WorkflowModel).filter_by(study_id=study_id).all() for workflow in workflows: if workflow.status == WorkflowStatus.user_input_required or workflow.status == WorkflowStatus.waiting: - processor = WorkflowProcessor(workflow) - processor.reset(workflow) + WorkflowProcessor.reset(workflow, clear_data=False) diff --git a/tests/test_lookup_service.py b/tests/test_lookup_service.py index 0f7d1c13..9b622fcd 100644 --- a/tests/test_lookup_service.py +++ b/tests/test_lookup_service.py @@ -54,8 +54,7 @@ class TestLookupService(BaseTest): # restart the workflow, so it can pick up the changes. - processor = WorkflowProcessor(workflow) - processor.reset(workflow) + processor = WorkflowProcessor.reset(workflow) workflow = processor.workflow_model LookupService.lookup(workflow, "Task_Enum_Lookup", "sponsor", "sam", limit=10) @@ -92,8 +91,7 @@ class TestLookupService(BaseTest): results = LookupService.lookup(workflow, task.task_spec.name, "selectedItem", "", value="apples", limit=10) self.assertEqual(0, len(results), "We shouldn't find our fruits mixed in with our animals.") - - processor.reset(workflow, clear_data=True) + processor = WorkflowProcessor.reset(workflow, clear_data=True) processor.do_engine_steps() task = processor.get_ready_user_tasks()[0] task.data = {"type": "fruit"} diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index 8a2b2b68..e3eda1a8 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -213,6 +213,27 @@ class TestTasksApi(BaseTest): self.assertTrue(workflow_api.spec_version.startswith("v2 ")) self.assertTrue(workflow_api.is_latest_spec) + def test_reset_workflow_from_broken_spec(self): + # Start the basic two_forms workflow and complete a task. + workflow = self.create_workflow('two_forms') + workflow_api = self.get_workflow_api(workflow) + self.complete_form(workflow, workflow_api.next_task, {"color": "blue"}) + self.assertTrue(workflow_api.is_latest_spec) + + # Break the bpmn json + workflow.bpmn_workflow_json = '{"something":"broken"}' + session.add(workflow) + session.commit() + + # Try to load the workflow, we should get an error + with self.assertRaises(Exception): + workflow_api = self.complete_form(workflow, workflow_api.next_task, {"name": "Dan"}) + + # Now, Reset the workflow, and we should not get an error + workflow_api = self.restart_workflow_api(workflow_api, clear_data=True) + self.assertIsNotNone(workflow_api) + + def test_manual_task_with_external_documentation(self): workflow = self.create_workflow('manual_task_with_external_documentation') diff --git a/tests/workflow/test_workflow_processor.py b/tests/workflow/test_workflow_processor.py index 21d9ba48..1d1813ed 100644 --- a/tests/workflow/test_workflow_processor.py +++ b/tests/workflow/test_workflow_processor.py @@ -279,7 +279,7 @@ class TestWorkflowProcessor(BaseTest): self.assertFalse(processor2.is_latest_spec) # Still at version 1. # Do a hard reset, which should bring us back to the beginning, but retain the data. - processor2.reset(processor2.workflow_model) + processor2 = WorkflowProcessor.reset(processor2.workflow_model) processor3 = WorkflowProcessor(processor.workflow_model) processor3.do_engine_steps() self.assertEqual("Step 1", processor3.next_task().task_spec.description) From 80ff8a6212987d5e44f872d10fb076ab0a353787 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 9 Mar 2021 13:31:26 -0500 Subject: [PATCH 4/5] Allows us to delete a study even if there are emails, and associated study users connected to that study. --- crc/services/study_service.py | 5 +++++ tests/study/test_study_api.py | 15 ++++++++++++--- tests/test_datastore_api.py | 4 ++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/crc/services/study_service.py b/crc/services/study_service.py index 48c326a6..c4576578 100644 --- a/crc/services/study_service.py +++ b/crc/services/study_service.py @@ -9,6 +9,7 @@ from ldap3.core.exceptions import LDAPSocketOpenError from crc import db, session, app from crc.api.common import ApiError +from crc.models.email import EmailModel from crc.models.file import FileDataModel, FileModel, FileModelSchema, File, LookupFileModel, LookupDataModel from crc.models.ldap import LdapSchema from crc.models.protocol_builder import ProtocolBuilderStudy, ProtocolBuilderStatus @@ -210,6 +211,10 @@ class StudyService(object): @staticmethod def delete_study(study_id): session.query(TaskEventModel).filter_by(study_id=study_id).delete() + session.query(StudyAssociated).filter_by(study_id=study_id).delete() + session.query(EmailModel).filter_by(study_id=study_id).delete() + session.query(StudyEvent).filter_by(study_id=study_id).delete() + for workflow in session.query(WorkflowModel).filter_by(study_id=study_id): StudyService.delete_workflow(workflow.id) study = session.query(StudyModel).filter_by(id=study_id).first() diff --git a/tests/study/test_study_api.py b/tests/study/test_study_api.py index c729f211..ec53e112 100644 --- a/tests/study/test_study_api.py +++ b/tests/study/test_study_api.py @@ -1,17 +1,19 @@ import json from profile import Profile +from crc.services.ldap_service import LdapService from tests.base_test import BaseTest from datetime import datetime, timezone from unittest.mock import patch +from crc.models.email import EmailModel from crc import session, app from crc.models.protocol_builder import ProtocolBuilderStatus, \ ProtocolBuilderStudySchema from crc.models.file import FileModel from crc.models.task_event import TaskEventModel -from crc.models.study import StudyEvent, StudyModel, StudySchema, StudyStatus, StudyEventType +from crc.models.study import StudyEvent, StudyModel, StudySchema, StudyStatus, StudyEventType, StudyAssociated from crc.models.workflow import WorkflowSpecModel, WorkflowModel from crc.services.file_service import FileService from crc.services.workflow_processor import WorkflowProcessor @@ -274,7 +276,7 @@ class TestStudyApi(BaseTest): self.assertTrue(file.archived) self.assertIsNone(file.workflow_id) - def test_delete_study_with_workflow_and_status(self): + def test_delete_study_with_workflow_and_status_etc(self): self.load_example_data() workflow = session.query(WorkflowModel).first() stats1 = StudyEvent( @@ -284,6 +286,14 @@ class TestStudyApi(BaseTest): event_type=StudyEventType.user, user_uid=self.users[0]['uid'], ) + LdapService.user_info('dhf8r') # Assure that there is a dhf8r in ldap for StudyAssociated. + + email = EmailModel(subject="x", study_id=workflow.study_id) + associate = StudyAssociated(study_id=workflow.study_id, uid=self.users[0]['uid']) + event = StudyEvent(study_id=workflow.study_id) + session.add_all([email, associate, event]) + + stats2 = TaskEventModel(study_id=workflow.study_id, workflow_id=workflow.id, user_uid=self.users[0]['uid']) session.add_all([stats1, stats2]) session.commit() @@ -293,7 +303,6 @@ class TestStudyApi(BaseTest): self.assertIsNone(del_study) - # """ # Workflow Specs that have been made available (or not) to a particular study via the status.bpmn should be flagged # as available (or not) when the list of a study's workflows is retrieved. diff --git a/tests/test_datastore_api.py b/tests/test_datastore_api.py index c9b3154a..6293701d 100644 --- a/tests/test_datastore_api.py +++ b/tests/test_datastore_api.py @@ -66,7 +66,7 @@ class DataStoreTest(BaseTest): - def test_update_study(self): + def test_update_datastore(self): self.load_example_data() new_study = self.add_test_study_data() new_study = session.query(DataStoreModel).filter_by(id=new_study["id"]).first() @@ -87,7 +87,7 @@ class DataStoreTest(BaseTest): - def test_delete_study(self): + def test_delete_datastore(self): self.load_example_data() new_study = self.add_test_study_data() oldid = new_study['id'] From 2ed825aef310b375b8c85e90babdbbc38848bc0a Mon Sep 17 00:00:00 2001 From: Kelly McDonald Date: Wed, 10 Mar 2021 10:59:15 -0500 Subject: [PATCH 5/5] Fixes problem with swtiching users fixes #237 --- crc/services/user_service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crc/services/user_service.py b/crc/services/user_service.py index 56a360ee..a8d45254 100644 --- a/crc/services/user_service.py +++ b/crc/services/user_service.py @@ -62,7 +62,7 @@ class UserService(object): if uid is None: raise ApiError("invalid_uid", "Please provide a valid user uid.") - if not UserService.admin_is_impersonating() and UserService.is_different_user(uid): + if UserService.is_different_user(uid): # Impersonate the user if the given uid is valid. impersonate_user = session.query(UserModel).filter(UserModel.uid == uid).first() @@ -70,6 +70,7 @@ class UserService(object): g.impersonate_user = impersonate_user # Store the uid and user session token. + session.query(AdminSessionModel).filter(AdminSessionModel.token==g.token).delete() session.add(AdminSessionModel(token=g.token, admin_impersonate_uid=uid)) session.commit() else: