diff --git a/battDB/forms.py b/battDB/forms.py index 8f2bf2da..e9713c15 100644 --- a/battDB/forms.py +++ b/battDB/forms.py @@ -330,11 +330,12 @@ class Meta: "binary_file", ] help_texts = { + "name": mark_safe("If left blank, the file name will be used."), "machine": mark_safe( "The machine this data file was collected on. " ' ' "new machine ⧉" - ) + ), } def __init__(self, *args, **kwargs): diff --git a/battDB/migrations/0032_alter_devicespecification_name_alter_equipment_name_and_more.py b/battDB/migrations/0032_alter_devicespecification_name_alter_equipment_name_and_more.py new file mode 100644 index 00000000..647d2509 --- /dev/null +++ b/battDB/migrations/0032_alter_devicespecification_name_alter_equipment_name_and_more.py @@ -0,0 +1,33 @@ +# Generated by Django 4.1.3 on 2022-11-29 15:52 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('battDB', '0031_experimentdatafile_binary_file_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='devicespecification', + name='name', + field=models.CharField(default='', max_length=128, unique=True), + ), + migrations.AlterField( + model_name='equipment', + name='name', + field=models.CharField(default='', max_length=128, unique=True), + ), + migrations.AlterField( + model_name='experiment', + name='name', + field=models.CharField(default='', max_length=128), + ), + migrations.AlterField( + model_name='parser', + name='name', + field=models.CharField(default='', max_length=128, unique=True), + ), + ] diff --git a/battDB/models.py b/battDB/models.py index 3f92dade..7fde6b35 100644 --- a/battDB/models.py +++ b/battDB/models.py @@ -19,7 +19,7 @@ from parsing_engines import available_parsing_engines, parse_data_file -class DeviceSpecification(cm.BaseModel, cm.HasMPTT): +class DeviceSpecification(cm.BaseModelMandatoryName, cm.HasMPTT): """A template for creating a device or batch of devices in the system.
Specifications are structured as a tree, so that each device can be composed of @@ -345,7 +345,7 @@ def get_number_parameters(self): return self.columns.count() -class Equipment(cm.BaseModel): +class Equipment(cm.BaseModelMandatoryName): """Definitions of equipment such as cycler machines.""" institution = models.ForeignKey( @@ -398,6 +398,13 @@ class Experiment(cm.BaseModel): ("other", "Other"), ) + name = models.CharField( + max_length=128, + blank=False, + default="", + null=False, + ) + date = models.DateField(default=datetime.now) config = models.ForeignKey( @@ -474,6 +481,24 @@ def __str__(self): def get_absolute_url(self): return reverse("battDB:Experiment", kwargs={"pk": self.pk}) + def clean(self): + """Validate that the name for an experiment is unique per institution. + + Args: + name: The name attempting to be saved. + + Raises: + ValidationError: _description_ + """ + instances = Experiment.objects.filter( + name=self.name, + user_owner__institution=self.user_owner.institution, + ) + if len(instances) > 0 and instances[0] != self: + raise ValidationError( + f"Name '{self.name}' is already used in your institution" + ) + class ExperimentDataFile(cm.BaseModel): """EDF is the class tying together data files, parsed data tables and experiments. diff --git a/common/models.py b/common/models.py index 33fc36f2..698cbd32 100644 --- a/common/models.py +++ b/common/models.py @@ -181,7 +181,9 @@ class Meta: class BaseModelMandatoryName(BaseModel): """Changed BaseModel.name to blank=False.""" - name = models.CharField(max_length=128, blank=False, default="", null=False) + name = models.CharField( + max_length=128, blank=False, default="", null=False, unique=True + ) class Meta: abstract = True diff --git a/common/views.py b/common/views.py index 789a57dd..8708ff88 100644 --- a/common/views.py +++ b/common/views.py @@ -19,10 +19,10 @@ def get(self, request, *args, **kwargs): def post(self, request, *args, **kwargs): form = self.form_class(request.POST, request.FILES) + form.instance.user_owner = request.user if form.is_valid(): obj = form.save(commit=False) # Do other stuff before saving here - obj.user_owner = request.user if form.is_public(): obj.status = "public" else: @@ -70,12 +70,13 @@ def get(self, request, *args, **kwargs): def post(self, request, *args, **kwargs): form = self.form_class(request.POST, request.FILES) + form.instance.user_owner = request.user context = self.get_context_data() + context["form"] = form # update form in context for form errors in render if form.is_valid(): # Save instance incluing setting user owner and status with transaction.atomic(): obj = form.save(commit=False) - obj.user_owner = request.user if form.is_public(): obj.status = "public" else: diff --git a/dfndb/migrations/0016_alter_compound_unique_together_alter_component_name_and_more.py b/dfndb/migrations/0016_alter_compound_unique_together_alter_component_name_and_more.py new file mode 100644 index 00000000..c7fa9b01 --- /dev/null +++ b/dfndb/migrations/0016_alter_compound_unique_together_alter_component_name_and_more.py @@ -0,0 +1,34 @@ +# Generated by Django 4.1.3 on 2022-11-29 11:28 + +from django.db import migrations, models + +import dfndb.validators + + +class Migration(migrations.Migration): + + dependencies = [ + ('dfndb', '0015_remove_compound_mass_alter_compound_formula'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='compound', + unique_together=set(), + ), + migrations.AlterField( + model_name='component', + name='name', + field=models.CharField(default='', max_length=128, unique=True), + ), + migrations.AlterField( + model_name='compound', + name='formula', + field=models.CharField(help_text='Chemical formula', max_length=20, unique=True, validators=[dfndb.validators.validate_formula]), + ), + migrations.AlterField( + model_name='compound', + name='name', + field=models.CharField(help_text='Full name for the element or compound.', max_length=100, unique=True), + ), + ] diff --git a/dfndb/models.py b/dfndb/models.py index 929774ee..04b88338 100644 --- a/dfndb/models.py +++ b/dfndb/models.py @@ -17,9 +17,13 @@ class Compound(models.Model): name = models.CharField( max_length=100, help_text="Full name for the element or compound.", + unique=True, ) formula = models.CharField( - max_length=20, help_text="Chemical formula", validators=[validate_formula] + max_length=20, + help_text="Chemical formula", + unique=True, + validators=[validate_formula], ) @property @@ -29,14 +33,8 @@ def mass(self): def __str__(self): return "%s (%s)" % (self.name, self.formula) - class Meta: - unique_together = ( - "name", - "formula", - ) - -class Component(cm.BaseModel): +class Component(cm.BaseModelMandatoryName): """A component used as part of an electrochemical cell specification, e.g. NMC622. Make use of the 'notes' field for additional explanation diff --git a/tests/battDB/baker_recipes.py b/tests/battDB/baker_recipes.py index 3acae8bd..e85293a2 100644 --- a/tests/battDB/baker_recipes.py +++ b/tests/battDB/baker_recipes.py @@ -1,4 +1,4 @@ -from model_bakery.recipe import Recipe, foreign_key +from model_bakery.recipe import Recipe, foreign_key, seq from battDB.models import ( Batch, @@ -22,7 +22,11 @@ from tests.dfndb.baker_recipes import component, parameter from tests.management.baker_recipes import user -device_specification = Recipe(DeviceSpecification, user_owner=foreign_key(user)) +device_specification = Recipe( + DeviceSpecification, + user_owner=foreign_key(user), + name=seq("Device Specification"), # Ensure uniqueness +) device_parameter = Recipe( DeviceParameter, @@ -57,11 +61,17 @@ parser = Recipe(Parser, user_owner=foreign_key(user)) equipment = Recipe( - Equipment, institution=foreign_key(org), user_owner=foreign_key(user) + Equipment, + institution=foreign_key(org), + user_owner=foreign_key(user), + name=seq("Equipment"), # Ensure uniqueness ) experiment = Recipe( - Experiment, config=foreign_key(device_config), user_owner=foreign_key(user) + Experiment, + config=foreign_key(device_config), + user_owner=foreign_key(user), + name=seq("Experiment"), # Ensure uniqueness ) edf = Recipe(ExperimentDataFile, user_owner=foreign_key(user)) diff --git a/tests/battDB/test_models.py b/tests/battDB/test_models.py index 5d5a4a1e..2fbb7b36 100644 --- a/tests/battDB/test_models.py +++ b/tests/battDB/test_models.py @@ -158,6 +158,8 @@ def test_definition(self): class TestExperiment(TestCase): def setUp(self): self.model = baker.make_recipe("tests.battDB.experiment") + self.model.user_owner.institution = baker.make_recipe("tests.common.org") + self.model.user_owner.save() def test_definition(self): self.assertTrue(hasattr(self.model, "date")) @@ -179,6 +181,17 @@ def test_get_absolute_url(self): url = self.model.get_absolute_url() self.assertIn("battDB/exps", url) + def test_unique_name(self): + with self.assertRaises(ValidationError): + new = baker.make_recipe( + "tests.battDB.experiment", + name=self.model.name, + user_owner=self.model.user_owner, + ) + new.clean() + another = baker.make_recipe("tests.battDB.experiment", name=self.model.name) + another.clean() + class TestExperimentDataFile(TestCase): def setUp(self): diff --git a/tests/battDB/test_views.py b/tests/battDB/test_views.py index 6d6502c0..2a0cb1eb 100644 --- a/tests/battDB/test_views.py +++ b/tests/battDB/test_views.py @@ -1,4 +1,5 @@ import tempfile +from datetime import datetime from django.contrib.auth.models import Group from django.test import TestCase, override_settings @@ -302,6 +303,84 @@ def test_detail_view(self): self.assertContains(response, "

test experiment

") +class CreateExperimentTest(TestCase): + def setUp(self): + self.user = baker.make_recipe( + "tests.management.user", + username="test_contributor", + ) + self.user.is_active = True + self.user.set_password("contribpass") + self.user.institution = baker.make_recipe("tests.common.org") + self.user.save() + group = Group.objects.get(name="Contributor") + group.user_set.add(self.user) + + def test_create_update_delete_experiment(self): + login_response = self.client.post( + "/accounts/login/", + {"username": "test_contributor", "password": "contribpass"}, + ) + self.assertEqual(login_response.status_code, 302) + self.assertEqual(login_response.url, "/") + + form_fields = { + "name": "Experiment 1", + "date": datetime.now().date(), + "exp_type": "constant", + "thermal": "none", + "summary": "Not long enough", + } + + # Invalid Form (short summary) + response = self.client.post(reverse("battDB:New Experiment"), form_fields) + with self.assertRaises(bdb.Experiment.DoesNotExist): + bdb.Experiment.objects.get() + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Ensure this value has at least 20 characters") + + # Create new experiment + form_fields["summary"] = "At least 20 characters" + response = self.client.post(reverse("battDB:New Experiment"), form_fields) + experiment = bdb.Experiment.objects.get(name="Experiment 1") + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, f"/battDB/exps/{experiment.id}/") + self.assertEqual(experiment.user_owner.username, "test_contributor") + self.assertEqual(experiment.user_owner.institution, self.user.institution) + self.assertEqual(experiment.status, "private") + for key, val in form_fields.items(): + self.assertEqual(getattr(experiment, key), val) + + # Create new experiment with same name - should fail + response = self.client.post(reverse("battDB:New Experiment"), form_fields) + self.assertEqual(len(bdb.Experiment.objects.all()), 1) + self.assertEqual(bdb.Experiment.objects.get(name="Experiment 1"), experiment) + self.assertEqual(response.status_code, 200) + self.assertContains(response, " is already used in your institution") + self.assertContains(response, form_fields["name"]) + + # Update experiment + form_fields["summary"] = "A longer and more detailed summary" + update_response = self.client.post( + reverse("battDB:Update Experiment", kwargs={"pk": experiment.id}), + form_fields, + ) + self.assertEqual(update_response.status_code, 302) + self.assertEqual(update_response.url, f"/battDB/exps/{experiment.id}/") + + experiment.refresh_from_db() + self.assertEqual(experiment.summary, "A longer and more detailed summary") + + # Delete experiment + delete_response = self.client.post( + reverse("battDB:Delete Experiment", kwargs={"pk": experiment.id}) + ) + self.assertEqual(delete_response.status_code, 302) + self.assertEqual(delete_response.url, "/battDB/exps/") + experiment.refresh_from_db() + self.assertEqual(experiment.status, "deleted") + + class DeviceSpecificationViewTest(TestCase): def setUp(self): self.user = baker.make_recipe( diff --git a/tests/dfndb/test_models.py b/tests/dfndb/test_models.py index ca49bdc5..28b06d0c 100644 --- a/tests/dfndb/test_models.py +++ b/tests/dfndb/test_models.py @@ -21,12 +21,19 @@ def test_str(self): f"{self.expected['name']} ({self.expected['formula']})", ) - def test_unique_together(self): - baker.make_recipe("tests.dfndb.compound", name="Carbon Dioxide", formula="CO3") + def test_unique(self): + from django.db import transaction + with self.assertRaises(IntegrityError): - baker.make_recipe( - "tests.dfndb.compound", name="Carbon Dioxide", formula="CO2" - ) + with transaction.atomic(): + baker.make_recipe( + "tests.dfndb.compound", name="Carbon Dioxide", formula="CO3" + ) + with self.assertRaises(IntegrityError): + with transaction.atomic(): + baker.make_recipe( + "tests.dfndb.compound", name="Carbon Dioxide 2", formula="CO2" + ) def test_invalid_formula(self): with self.assertRaises(ValidationError):