From 41481bf4ad2c0a56fb39c9abdc1e8e72ad4cc649 Mon Sep 17 00:00:00 2001 From: Anne Haley Date: Thu, 25 Aug 2022 13:55:09 -0400 Subject: [PATCH] Bug hunt 4 (#567) * Fix scan states for task overview * Clean up logout method references * Use image value distribution to remove outliers from automatic window values * Prevent experiments from getting added to the experiment list twice * Fix change to logout button in Navbar * Remove console log * Update project state when a scan decision is submitted * Reformat email content * Use timeout dialog when server session has expired * Remove vtk view drag interaction for updating window * Strip whitespace values from paths in import files * Account for undefined values in various places, as reported by Sentry * Demo instance does not have access to .git directory and cannot fetch version * Write Github action for running ansible playbook (#568) * Write Github action for running ansible playbook * Remove "on pull request" * Use -p argument for mkdir command Co-authored-by: Zach Mullen Co-authored-by: Zach Mullen * Use a fixed number of queries for Project.get_status() * Add test for project status response * Use project.get_status directly and zip(decisions, scans) * Remove refresh from DB Co-authored-by: Zach Mullen --- .github/workflows/ansible.yml | 27 ++++++++++++++ miqa/core/conversion/import_export_csvs.py | 2 +- miqa/core/models/project.py | 32 ++++++++--------- miqa/core/rest/other_endpoints.py | 16 +++++---- miqa/core/rest/project.py | 2 +- miqa/core/signals.py | 35 ++++++++++--------- miqa/core/tests/test_rest.py | 29 ++++++++++++++- web_client/src/components/DecisionButtons.vue | 12 ++++++- .../src/components/KeyboardShortcutDialog.vue | 3 -- web_client/src/components/Navbar.vue | 10 +++--- web_client/src/components/TimeoutDialog.vue | 31 ++++++++++------ web_client/src/components/UserButton.vue | 2 +- web_client/src/components/VtkViewer.vue | 27 +++++++------- web_client/src/components/WindowWidget.vue | 15 ++++++-- web_client/src/django.ts | 5 +-- web_client/src/store/index.ts | 10 +++--- web_client/src/utils/crosshairs.js | 1 + web_client/src/utils/fill2DView.js | 1 + web_client/src/views/Frame.vue | 5 --- web_client/src/views/Projects.vue | 6 ++-- 20 files changed, 174 insertions(+), 97 deletions(-) create mode 100644 .github/workflows/ansible.yml diff --git a/.github/workflows/ansible.yml b/.github/workflows/ansible.yml new file mode 100644 index 00000000..bd286fea --- /dev/null +++ b/.github/workflows/ansible.yml @@ -0,0 +1,27 @@ +name: Update Celery worker +on: + push: + branches: + - master + +jobs: + ansible: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Install Ansible role + run: ansible-galaxy install -r ansible/requirements.yml + - name: Write vault pass and private key + env: + VAULT_PASSWORD: ${{ secrets.ANSIBLE_VAULT_PASSWORD }} + PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }} + run: | + echo $VAULT_PASSWORD > ansible/vault_pass.txt + mkdir -p $HOME/.ssh + echo "$PRIVATE_KEY" > $HOME/.ssh/id_miqa + chmod 600 $HOME/.ssh/id_miqa + - name: Run Ansible playbook for Celery + env: + ANSIBLE_HOST_KEY_CHECKING: false + working-directory: ansible + run: ansible-playbook --vault-password-file vault_pass.txt -i hosts playbook.yml diff --git a/miqa/core/conversion/import_export_csvs.py b/miqa/core/conversion/import_export_csvs.py index 78db60c5..696adda4 100644 --- a/miqa/core/conversion/import_export_csvs.py +++ b/miqa/core/conversion/import_export_csvs.py @@ -34,7 +34,7 @@ def validate_file_locations(input_dict, project, not_found_errors): import_path = GlobalSettings.load().import_path if project is None else project.import_path for key, value in input_dict.items(): if key == 'file_location': - raw_path = Path(value) + raw_path = Path(value.strip()) if not value.startswith('s3://'): if not raw_path.is_absolute(): # not an absolute file path; refer to project import csv location diff --git a/miqa/core/models/project.py b/miqa/core/models/project.py index 8aef200e..4f24c5c4 100644 --- a/miqa/core/models/project.py +++ b/miqa/core/models/project.py @@ -97,25 +97,23 @@ def get_status(self): tier_2_reviewers = [ user.id for user in get_users_with_perms(self, only_with_perms_in=['tier_2_reviewer']) ] - scans_in_project = ( - Scan.objects.filter(experiment__in=self.experiments.all()) - .prefetch_related('decisions') - .annotate(decision_count=models.Count('decisions')) - ) - complete_scans_in_project = ( - scans_in_project.filter(decision_count__gt=0) - .annotate( - last_decider_id=models.Subquery( - ScanDecision.objects.filter(scan__id=models.OuterRef('id')) - .order_by('-created') - .values('creator_id')[:1] - ) - ) - .filter(last_decider_id__in=tier_2_reviewers) - ) + scans_in_project = Scan.objects.filter(experiment__project=self) + completed_scans_in_project = scans_in_project.alias( + latest_decider_id=models.Subquery( + ScanDecision.objects.filter(scan__id=models.OuterRef('id')) + .order_by('-created') + .values('creator_id')[:1] + ), + latest_decision=models.Subquery( + ScanDecision.objects.filter(scan__id=models.OuterRef('id')) + .order_by('-created') + .values('decision')[:1] + ), + ).filter(models.Q(latest_decider_id__in=tier_2_reviewers) | models.Q(latest_decision='U')) + return { 'total_scans': scans_in_project.count(), - 'total_complete': complete_scans_in_project.count(), + 'total_complete': completed_scans_in_project.count(), } def update_group(self, group_name, user_list): diff --git a/miqa/core/rest/other_endpoints.py b/miqa/core/rest/other_endpoints.py index c6763d28..9d1b9750 100644 --- a/miqa/core/rest/other_endpoints.py +++ b/miqa/core/rest/other_endpoints.py @@ -13,13 +13,15 @@ capture_output=True, ).stdout.decode() -MIQA_VERSION += ( - ' commit: ' - + subprocess.run( - ['git', 'rev-parse', 'HEAD'], - capture_output=True, - ).stdout.decode() -) +# Heroku doesn't have access to .git directory, version will be blank +if len(MIQA_VERSION) > 0: + MIQA_VERSION += ( + ' commit: ' + + subprocess.run( + ['git', 'rev-parse', 'HEAD'], + capture_output=True, + ).stdout.decode() + ) class MIQAConfigView(APIView): diff --git a/miqa/core/rest/project.py b/miqa/core/rest/project.py index 23949ec6..6814f3f7 100644 --- a/miqa/core/rest/project.py +++ b/miqa/core/rest/project.py @@ -91,7 +91,7 @@ def convert_state_string(last_decision): return { str(scan.id): convert_state_string(scan.decisions.latest('created')) - if scan.decisions.count() > 0 and scan.decisions.latest('created').creator + if scan.decisions.count() > 0 else 'unreviewed' for exp in obj.experiments.all() for scan in exp.scans.all() diff --git a/miqa/core/signals.py b/miqa/core/signals.py index a33a130b..2da97fbb 100644 --- a/miqa/core/signals.py +++ b/miqa/core/signals.py @@ -20,23 +20,26 @@ def require_admin_approval(sender, **kwargs): 'account-activate', args=[kwargs['email_address']], request=kwargs['request'] ) - email_content = f'A new user with the email {kwargs["email_address"]} has created ' - 'an account in MIQA. As an administrative user, it is your responsibility ' - 'to activate this account if you believe this user is legitimate. ' - 'If you believe this account should not be activated, reach out to this user' - f'{" and other administrators." if len(admins) > 1 else "."} ' - '\n\n ' - f'To activate this account, visit {activation_link} ' - '\n\n ' - 'Thank you for using MIQA! ' + email_content = ( + f'A new user with the email {kwargs["email_address"]} has created ' + 'an account in MIQA. As an administrative user, it is your responsibility ' + 'to activate this account if you believe this user is legitimate. ' + 'If you believe this account should not be activated, reach out to this user' + f'{" and other administrators." if len(admins) > 1 else "."} ' + '\n\n ' + f'To activate this account, visit {activation_link} ' + '\n\n ' + 'Thank you for using MIQA! ' + ) else: - email_content = f'A new user with the email {kwargs["email_address"]} has created ' - 'an account in MIQA. Since this instance is in demo mode, you do not need to ' - 'activate this account. This email is a notification of the change; ' - 'no further action is required.' - '\n\n ' - 'Thank you for using MIQA! ' - + email_content = ( + f'A new user with the email {kwargs["email_address"]} has created ' + 'an account in MIQA. Since this instance is in demo mode, you do not need to ' + 'activate this account. This email is a notification of the change; ' + 'no further action is required.' + '\n\n ' + 'Thank you for using MIQA! ' + ) msg = EmailMultiAlternatives( 'MIQA Account Approval - New User', email_content, diff --git a/miqa/core/tests/test_rest.py b/miqa/core/tests/test_rest.py index abd34c3a..1ef31d56 100644 --- a/miqa/core/tests/test_rest.py +++ b/miqa/core/tests/test_rest.py @@ -1,7 +1,7 @@ import json from uuid import UUID -from guardian.shortcuts import get_perms +from guardian.shortcuts import assign_perm, get_perms import pytest from miqa.core.rest.frame import FrameSerializer @@ -62,6 +62,33 @@ def test_projects_list(user_api_client, project, user): } +@pytest.mark.django_db +def test_project_status( + project, + experiment, + scan_factory, + scan_decision_factory, + user_factory, +): + # 3 of 5 scans marked as complete + decisions = [ + ('U', 'tier_1_reviewer'), + ('U', 'tier_2_reviewer'), + ('UN', 'tier_1_reviewer'), # needs tier 2 review + ('UN', 'tier_2_reviewer'), + ('Q?', 'tier_1_reviewer'), # needs tier 2 review + ] + scans = [scan_factory(experiment=experiment) for i in range(len(decisions))] + for decision, scan in zip(decisions, scans): + decider = user_factory() + assign_perm(decision[1], decider, project) + scan_decision_factory(scan=scan, creator=decider, decision=decision[0]) + # project.refresh_from_db() + status = project.get_status() + assert status['total_scans'] == len(scans) + assert status['total_complete'] == 3 + + @pytest.mark.django_db def test_project_settings_get(user_api_client, project, user): resp = user_api_client().get(f'/api/v1/projects/{project.id}/settings') diff --git a/web_client/src/components/DecisionButtons.vue b/web_client/src/components/DecisionButtons.vue index c5ba516a..4db329cc 100644 --- a/web_client/src/components/DecisionButtons.vue +++ b/web_client/src/components/DecisionButtons.vue @@ -48,6 +48,7 @@ export default { computed: { ...mapState([ 'currentViewData', + 'currentProject', 'proxyManager', 'vtkViews', 'storeCrosshairs', @@ -66,7 +67,7 @@ export default { return this.artifacts.map((artifact) => [artifact, this.getCurrentChipState(artifact)]); }, suggestedArtifacts() { - if (this.currentViewData.scanDecisions.length > 0) { + if (this.currentViewData.scanDecisions && this.currentViewData.scanDecisions.length > 0) { const lastDecision = _.sortBy( this.currentViewData.scanDecisions, (dec) => dec.created, )[0]; @@ -221,6 +222,14 @@ export default { this.confirmedPresent.push(artifact.value); } }, + async refreshTaskOverview() { + if (this.currentProject) { + const taskOverview = await djangoRest.projectTaskOverview(this.currentProject.id); + if (JSON.stringify(store.state.currentTaskOverview) !== JSON.stringify(taskOverview)) { + store.commit.setTaskOverview(taskOverview); + } + } + }, handleCommentChange(value) { this.newComment = value; }, @@ -255,6 +264,7 @@ export default { currentScan: this.currentViewData.scanId, newDecision: savedObj, }); + this.refreshTaskOverview(); this.$emit('handleKeyPress', 'next'); this.$snackbar({ text: 'Saved decision successfully.', diff --git a/web_client/src/components/KeyboardShortcutDialog.vue b/web_client/src/components/KeyboardShortcutDialog.vue index 04ec70e4..b472302a 100644 --- a/web_client/src/components/KeyboardShortcutDialog.vue +++ b/web_client/src/components/KeyboardShortcutDialog.vue @@ -21,9 +21,6 @@ export default defineComponent({ ['z', 'x'], ], ], - ['Decrease/increase window width', [['-', '=']]], - ['Decrease/increase window level', [['[', ']']]], - ['Changing window width & level', [['click + dragging']]], ['Toggle fullscreen', [['e', 'd', 'c']]], ['Mark as usable/usable extra/questionable/unusable', [['u', 'i', 'o', 'p']]], ['Place crosshairs at location', [['click']]], diff --git a/web_client/src/components/Navbar.vue b/web_client/src/components/Navbar.vue index acd29bf5..8f892739 100644 --- a/web_client/src/components/Navbar.vue +++ b/web_client/src/components/Navbar.vue @@ -1,5 +1,5 @@