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 <[email protected]> Co-authored-by: Zach Mullen <[email protected]> * 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 <[email protected]>
OpenImaging · Aug 25, 2022 · 41481bf · 41481bf
1 parent 61cce38
commit 41481bf
Show file tree

Hide file tree

Showing 20 changed files with 174 additions and 97 deletions.
diff --git a/.github/workflows/ansible.yml 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,5 +1,5 @@
 <script lang="ts">
-import { mapActions, mapState, mapGetters } from 'vuex';
+import { mapState, mapGetters } from 'vuex';
 import { defineComponent } from '@vue/composition-api';
 
 import UserButton from '@/components/UserButton.vue';
@@ -8,6 +8,7 @@ import TimeoutDialog from '@/components/TimeoutDialog.vue';
 import EmailDialog from '@/components/EmailDialog.vue';
 import KeyboardShortcutDialog from '@/components/KeyboardShortcutDialog.vue';
 import UserAvatar from '@/components/UserAvatar.vue';
+import djangoClient from '@/django';
 
 export default defineComponent({
   name: 'Navbar',
@@ -47,9 +48,8 @@ export default defineComponent({
     },
   },
   methods: {
-    ...mapActions(['logout']),
     async logoutUser() {
-      await this.logout();
+      await djangoClient.logout();
       this.$router.go('/'); // trigger re-render into oauth flow
     },
     openDocumentation() {
@@ -71,7 +71,7 @@ export default defineComponent({
         <template #activator="{ on }">
           <span v-on="on">MIQA</span>
         </template>
-        <span>{{ MIQAConfig.version }}</span>
+        <span>{{ MIQAConfig.version.length > 0 ? MIQAConfig.version : "Demo Instance" }}</span>
       </v-tooltip>
     </v-toolbar-title>
     <v-tabs
@@ -134,7 +134,7 @@ export default defineComponent({
       :target-user="user"
     />
     <UserButton
-      @user="logoutUser()"
+      @logout="logoutUser()"
       @login="djangoRest.login()"
     />
     <TimeoutDialog />

diff --git a/web_client/src/components/TimeoutDialog.vue b/web_client/src/components/TimeoutDialog.vue
@@ -17,6 +17,7 @@ export default defineComponent({
   setup() {
     const show = ref(false);
     const idleWarningTriggered = ref(false);
+    const unauthorizedTriggered = ref(false);
     const idleStartTime = ref(0);
     const timeRemaining = ref(0);
     const timeRemainingStr = computed(() => {
@@ -26,25 +27,28 @@ export default defineComponent({
       return `${minutes}:${seconds}`;
     });
 
+    window.addEventListener(
+      'unauthorized', () => {
+        show.value = true;
+        unauthorizedTriggered.value = true;
+        timeRemaining.value = 30000;
+      },
+    );
+
     const lastApiRequestTime = computed(() => store.state.lastApiRequestTime);
 
-    const reset = () => {
-      // Send a request to refresh the server token
-      // Not awaited since we don't actually care about the result
+    const reset = async () => {
+      await djangoRest.restoreLogin(store);
       djangoRest.projects();
 
       // reset dialog
       show.value = false;
+      timeRemaining.value = 0;
       idleWarningTriggered.value = false;
+      unauthorizedTriggered.value = false;
     };
     const logout = async () => {
-      try {
-        // This may fail with a 401 if the user has already been logged out
-        await djangoRest.logout();
-      } finally {
-        // This will redirect to the login page
-        await djangoRest.login();
-      }
+      await djangoRest.logout();
     };
 
     // Watch for an absence of user interaction
@@ -68,6 +72,8 @@ export default defineComponent({
         // If the user is idle, we also need to consider the idle warning
         const idleTimeRemaining = idleStartTime.value + warningDuration - now;
         timeRemaining.value = Math.min(sessionTimeRemaining, idleTimeRemaining);
+      } else if (unauthorizedTriggered.value) {
+        timeRemaining.value -= 1000;
       } else {
         timeRemaining.value = sessionTimeRemaining;
       }
@@ -87,6 +93,7 @@ export default defineComponent({
     return {
       show,
       idleWarningTriggered,
+      unauthorizedTriggered,
       timeRemaining,
       timeRemainingStr,
       reset,
@@ -113,6 +120,10 @@ export default defineComponent({
         <p v-if="idleWarningTriggered">
           You have been idle for almost {{ Math.floor(idleTimeout / (60 * 1000)) }} minutes.
         </p>
+        <p v-else-if="unauthorizedTriggered">
+          Your server session has exceeded
+          {{ Math.floor(sessionTimeout / (60 * 1000)) }} minutes; your request has failed.
+        </p>
         <p v-else>
           You have not made any network requests in almost
           {{ Math.floor(sessionTimeout / (60 * 1000)) }} minutes.

diff --git a/web_client/src/components/UserButton.vue b/web_client/src/components/UserButton.vue
@@ -19,7 +19,7 @@ export default defineComponent({
     icon
     color="black lighten-1"
     class="mx-4"
-    @click="$emit('user')"
+    @click="$emit('logout')"
   >
     Logout
   </v-btn>