Merge pull request #633 from ucb-rit/develop

Enable admins to validate billing IDs; clarify why PIs may not be selected during new project request flow

bootstrap/development/docker/README.md

@@ -33,6 +33,7 @@ Note that these steps must be run from the root directory of the repo.
    Notes:
      - This step may be performed multiple times.
      - The `main.yml` file does not need to be modified in any way, despite indications within it. Its pre-defined values will be overridden and added to based on the other YML files in the directory.
+     - Settings may be added or overridden by specifying them in an `overrides.yml` file in the directory.
 
 5. Generate a `.env` file with environment variables that will be passed to `docker-compose.yml`. You must provide a deployment name ("BRC" or "LRC"), as well as a port where the web service will be available ("8880", "8881", "8882", or "8883").
 

coldfront/core/billing/forms.py

@@ -3,6 +3,7 @@
 from django.contrib.auth.models import User
 from django.core.validators import MinLengthValidator
 from django.core.validators import RegexValidator
+from django.utils.safestring import mark_safe
 
 from coldfront.core.billing.models import BillingActivity
 from coldfront.core.billing.utils.queries import get_billing_activity_from_full_id
@@ -66,7 +67,19 @@ def clean_billing_id(self):
         self.validate_billing_id(
             billing_id, ignore_invalid=not self.enforce_validity)
         return billing_id
-
+
+class BillingIDValidateManyForm(forms.Form):
+    billing_ids = forms.CharField(
+        help_text=mark_safe('Example:<br />123456-789<br />987654-321'),
+        label='Project IDs',
+        required=True,
+        widget=forms.Textarea(
+            attrs={'placeholder': 'Put each Project ID on its own line.'}
+        ))
+
+    def clean(self):
+        cleaned_data = super().clean()
+        return cleaned_data
 
 class BillingIDCreationForm(BillingIDValidityMixin, forms.Form):
 

coldfront/core/billing/management/commands/billing_ids.py

@@ -23,7 +23,7 @@
 
 class Command(BaseCommand):
 
-    help = 'Create and set billing IDs.'
+    help = 'Create, set, or validate billing IDs.'
 
     logger = logging.getLogger(__name__)
 
@@ -37,6 +37,7 @@ def add_arguments(self, parser):
         self._add_create_subparser(subparsers)
         self._add_list_subparser(subparsers)
         self._add_set_subparser(subparsers)
+        self._add_validate_subparser(subparsers)
 
     def handle(self, *args, **options):
         """Call the handler for the provided subcommand."""
@@ -104,6 +105,18 @@ def _add_set_subparser(parsers):
         add_ignore_invalid_argument(user_account_parser)
         add_argparse_dry_run_argument(user_account_parser)
 
+    @staticmethod
+    def _add_validate_subparser(parsers):
+        parser = parsers.add_parser(
+            'validate', help=(
+                'Check whether one or more billing IDs are valid.'))
+
+        parser.add_argument(
+            'billing_ids', 
+            help=('A space-separated list of billing IDs.'),
+            nargs='+',
+            type=str)
+
     @staticmethod
     def _get_billing_activity_or_error(full_id):
         """Return the BillingActivity corresponding to the given
@@ -262,6 +275,17 @@ def _handle_set(self, *args, **options):
             user = self._get_user_or_error(options['username'])
             self._handle_set_user_account(
                 user, billing_activity, dry_run=dry_run)
+
+    def _handle_validate(self, *args, **options):
+        """Handle the 'validate' subcommand."""
+        for full_id in options['billing_ids']:
+            if is_billing_id_well_formed(full_id):
+                if is_billing_id_valid(full_id):
+                    self.stdout.write(self.style.SUCCESS(full_id + ': Valid'))
+                else:
+                    self.stderr.write(self.style.ERROR(full_id + ': Invalid'))
+            else:
+                self.stderr.write(self.style.ERROR(full_id + ': Malformed'))
 
     def _handle_set_project_default(self, project, billing_activity,
                                     dry_run=False):

coldfront/core/billing/templates/billing/billing_id_usages_search.html

@@ -20,6 +20,10 @@ <h1>LBL Project ID Usages</h1>
   <i class="fas fa-plus" aria-hidden="true"></i>
   Create
 </a>
+<a class="class btn btn-primary" href="{% url 'billing-id-validate' %}">
+  <i class="fas fa-check" aria-hidden="true"></i>
+  Validate
+</a>
 <hr>
 {% endif %}
 

coldfront/core/billing/templates/billing/billing_id_validate.html

@@ -0,0 +1,31 @@
+{% extends "common/base.html" %}
+{% load common_tags %}
+{% load crispy_forms_tags %}
+{% load static %}
+
+{% block title %}
+  Validate LBL Project IDs
+{% endblock %}
+
+{% block content %}
+
+<h1>Validate LBL Project IDs</h1>
+<hr>
+
+<p>
+    To test the validity of multiple Project IDs, put each on its own line. 
+</p>
+
+<form method="post">
+  {% csrf_token %}
+  {{ form|crispy }}
+  <input type="submit" class="btn btn-primary" value="Validate">
+</form>
+
+<script>
+  $("#navbar-main > ul > li.active").removeClass("active");
+  $("#navbar-admin").addClass("active");
+  $("#navbar-billing-id-usages").addClass("active");
+</script>
+
+{% endblock %}

coldfront/core/billing/tests/test_commands/test_billing_ids.py

@@ -106,6 +106,28 @@ def test_create_success(self):
         billing_activity = get_billing_activity_from_full_id(billing_id)
         self.assertTrue(isinstance(billing_activity, BillingActivity))
 
+    def test_validate_success(self):
+        """Test that, given a variety of billing IDs, the
+        'validate' outputs correctly."""
+
+        malformed_billing_id = '12345-67'
+        self.assertFalse(is_billing_id_well_formed(malformed_billing_id))
+
+        invalid_billing_id = '123456-789'
+        self.assertTrue(is_billing_id_well_formed(invalid_billing_id))
+        self.assertFalse(is_billing_id_valid(invalid_billing_id))
+
+        valid_billing_id = '123456-788'
+        self.assertTrue(is_billing_id_well_formed(valid_billing_id))
+        self.assertTrue(is_billing_id_valid(valid_billing_id))
+
+        output, error = self.command.validate(
+            [malformed_billing_id, invalid_billing_id, valid_billing_id])
+
+        self.assertIn(malformed_billing_id + ': Malformed', error)
+        self.assertIn(invalid_billing_id + ': Invalid', error)
+        self.assertIn(valid_billing_id + ': Valid', output)
+
     # TODO: test_list
 
     @enable_deployment('LRC')
@@ -402,6 +424,15 @@ def create(self, billing_id, **flags):
         args = ['create', billing_id]
         self._add_flags_to_args(args, **flags)
         return self.call_subcommand(*args)
+
+    def validate(self, billing_ids, **flags):
+        """Call the 'validate' subcommand with the given billing IDs, and
+        flag values."""
+        args = ['validate']
+        for billing_id in billing_ids:
+            args.append(billing_id)
+        self._add_flags_to_args(args, **flags)
+        return self.call_subcommand(*args)
 
     def set_project_default(self, project_name, billing_id, **flags):
         """Call the 'project_default' subcommand of the 'set' subcommand

coldfront/core/billing/tests/test_views/test_billing_id_validate_view.py

@@ -0,0 +1,75 @@
+from django.urls import reverse
+from django.contrib.messages import get_messages
+
+from coldfront.core.billing.tests.test_billing_base import TestBillingBase
+from coldfront.core.billing.tests.test_commands.test_billing_ids import BillingIdsCommand
+from coldfront.core.billing.utils.queries import get_billing_activity_from_full_id
+from coldfront.core.billing.utils.queries import is_billing_id_well_formed
+from coldfront.core.billing.utils.validation import is_billing_id_valid
+
+from http import HTTPStatus
+
+class TestBillingIDValidateView(TestBillingBase):
+    """A class for testing BillingIDValidateView."""
+
+    def setUp(self):
+        """Set up test data."""
+        super().setUp()
+        self.create_test_user()
+        self.sign_user_access_agreement(self.user)
+        self.client.login(username=self.user.username, password=self.password)
+
+        self.url = reverse('billing-id-validate')
+
+        self.command = BillingIdsCommand()
+
+    def test_permissions_get(self):
+        """Test that the correct users have permissions to perform GET
+        requests."""
+
+        # Unauthenticated user.
+        self.client.logout()
+        response = self.client.get(self.url)
+        self.assert_redirects_to_login(response, next_url=self.url)
+
+        # Non superuser
+        self.client.login(username=self.user.username, password=self.password)
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN)
+
+        # Superuser
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, HTTPStatus.OK)
+        self.user.is_superuser = False
+        self.user.save()
+
+    def test_billing_ids_correctness(self):
+        """Test that, given a variety of billing IDs, the
+            'validate' outputs correctly."""
+
+        self.user.is_superuser = True
+        self.user.save()
+
+        malformed_billing_id = '12345-67'
+        self.assertFalse(is_billing_id_well_formed(malformed_billing_id))
+
+        invalid_billing_id = '123456-789'
+        self.assertTrue(is_billing_id_well_formed(invalid_billing_id))
+        self.assertFalse(is_billing_id_valid(invalid_billing_id))
+
+        valid_billing_id = '123456-788'
+        self.assertTrue(is_billing_id_well_formed(valid_billing_id))
+        self.assertTrue(is_billing_id_valid(valid_billing_id))
+
+        billing_ids = malformed_billing_id + '\n' + invalid_billing_id + \
+                        '\n' + valid_billing_id
+        response = self.client.post(self.url, data={'billing_ids': billing_ids})
+        messages = list(get_messages(response.wsgi_request))
+
+        self.assertEqual(len(messages), 1)
+
+        self.assertIn(malformed_billing_id + ': Malformed', messages[0].message)
+        self.assertIn(invalid_billing_id + ': Invalid', messages[0].message)
+        self.assertIn(valid_billing_id + ': Valid', messages[0].message)

coldfront/core/billing/urls.py

@@ -17,6 +17,9 @@
         path('usages/',
              admin_views.BillingIDUsagesSearchView.as_view(),
              name='billing-id-usages'),
+        path('validate/',
+             admin_views.BillingIDValidateView.as_view(),
+             name='billing-id-validate'),
     ]
 
 

coldfront/core/billing/views/admin_views.py

@@ -10,8 +10,10 @@
 from django.urls import reverse
 from django.views.generic.base import TemplateView
 from django.views.generic.edit import FormView
+from django.utils.safestring import mark_safe
 
 from coldfront.core.billing.forms import BillingIDCreationForm
+from coldfront.core.billing.forms import BillingIDValidateManyForm
 from coldfront.core.billing.forms import BillingIDSetProjectDefaultForm
 from coldfront.core.billing.forms import BillingIDSetRechargeForm
 from coldfront.core.billing.forms import BillingIDSetUserAccountForm
@@ -21,7 +23,10 @@
 from coldfront.core.billing.utils.billing_activity_managers import ProjectUserBillingActivityManager
 from coldfront.core.billing.utils.billing_activity_managers import UserBillingActivityManager
 from coldfront.core.billing.utils.queries import get_billing_id_usages
+from coldfront.core.billing.utils.queries import is_billing_id_well_formed
+from coldfront.core.billing.utils.queries import get_billing_activity_from_full_id
 from coldfront.core.billing.utils.queries import get_or_create_billing_activity_from_full_id
+from coldfront.core.billing.utils.validation import is_billing_id_valid
 from coldfront.core.project.models import Project
 from coldfront.core.project.models import ProjectUser
 
@@ -308,3 +313,41 @@ def get_context_data(self, **kwargs):
         context['next_url_parameter'] = urlencode({'next': next_url})
 
         return context
+
+class BillingIDValidateView(LoginRequiredMixin, UserPassesTestMixin, FormView):
+
+    form_class = BillingIDValidateManyForm
+    template_name = 'billing/billing_id_validate.html'
+
+    def test_func(self):
+        return self.request.user.is_superuser
+
+    def form_valid(self, form):
+        billing_ids = form.cleaned_data.get('billing_ids')
+
+        validation_results = ''
+        if billing_ids:
+            separated_billing_ids = [s.strip() for s in billing_ids.split('\n')]
+            exists_malformed_or_invalid = False
+            for full_id in separated_billing_ids:
+                if is_billing_id_well_formed(full_id):
+                    if is_billing_id_valid(full_id):
+                        append_str = "Valid"
+                    else:
+                        append_str = "Invalid"
+                        exists_malformed_or_invalid = True
+                else:
+                    append_str = "Malformed"
+                    exists_malformed_or_invalid = True
+
+                validation_results += f'{full_id}: {append_str}<br />' 
+        if exists_malformed_or_invalid:
+            messages.error(self.request, mark_safe(validation_results))
+        else:
+            messages.success(self.request, mark_safe(validation_results))
+
+        return super().form_valid(form)
+
+
+    def get_success_url(self):
+        return '/billing/validate/'

coldfront/core/project/templates/project/project_request/savio/project_existing_pi.html

@@ -27,15 +27,28 @@ <h1>{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator</h1><hr>
   <li>Allocation Period: {{ allocation_period.name }}</li>
 </ol>
 
-<p>Select an existing user to be a Principal Investigator of the project. You may search for the user in the selection field. If the desired user is not listed, you may skip this step and specify information for a new PI in the next step.</p>
-{% flag_enabled 'LRC_ONLY' as lrc_only %}
-{% if lrc_only %}
-  <p>Note: Only LBL employees, users with an "@lbl.gov" email, can be selected as a PI.</p>
-{% endif %}
+<p>Select a user to be a Principal Investigator (PI) of the project. <b>Please review the instructions below.</b></p>
 
-{% if allowance_is_one_per_pi %}
-  <p>Note: Each PI may only have one {{ computing_allowance.name }} at a time, so any that have pending requests or active allocations are not selectable.</p>
-{% endif %}
+<ul>
+  <li>If your PI is not listed, leave the field blank and provide information about your PI in the next step.</li>
+
+  {% flag_enabled 'LRC_ONLY' as lrc_only %}
+  {% if allowance_is_one_per_pi or lrc_only %}
+  <li>
+    If your PI is listed but not selectable, this may be for one of the following reasons:
+    <ul>
+      {% if allowance_is_one_per_pi %}
+      <li>A PI may only have one {{ computing_allowance.name }} at a time. Any PI with an existing {{ computing_allowance.name }} project or a pending request to create one is not selectable.</li>
+      {% endif %}
+      {% if lrc_only %}
+      <li>The PI must be an LBL employee. Any user who does not have an "@lbl.gov" email is not selectable.</li>
+      {% endif %}
+    </ul>
+  </li>
+  {% endif %}
+
+  <li>If you are not the PI, please do not select yourself.</li>
+</ul>
 
 <form action="" method="post">
   {% csrf_token %}
@@ -50,6 +63,15 @@ <h1>{{ PRIMARY_CLUSTER_NAME }}: Principal Investigator</h1><hr>
       {{ wizard.form|crispy }}
     {% endif %}
   </table>
+
+    <script>
+    $('select').selectize({
+      create: false,
+      sortField: 'text',
+      placeholder: 'Type to search. If your PI is not listed or selectable, please review the instructions above.'
+    })
+  </script>
+
   {% if wizard.steps.prev %}
     <button
         class="btn btn-secondary"