Skip to content

Commit

Permalink
Merge branch 'issue-3589-2' of https://github.com/specify/specify7 in…
Browse files Browse the repository at this point in the history
…to issue-3589-2
  • Loading branch information
alesan99 committed Oct 28, 2024
2 parents 8501fec + 77c9662 commit 5d547c6
Show file tree
Hide file tree
Showing 112 changed files with 2,631 additions and 24,021 deletions.
2 changes: 1 addition & 1 deletion .env
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ LOG_LEVEL=WARNING
# should only be used during development and troubleshooting and not
# during general use. Django applications leak memory when operated
# continuously in debug mode.
SP7_DEBUG=true
SP7_DEBUG=true
9 changes: 5 additions & 4 deletions .github/ISSUE_TEMPLATE/bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Steps to reproduce the behavior:
3. Scroll down to '....'
4. See error

*When reporting an issue on the `production` branch or any development version of Specify, please try to recreate the issue on the latest tagged release (`v7`) and indicate if it does not occur there, indicating a regression.*

**Expected behavior**
A clear and concise description of what you expected to happen.

Expand All @@ -26,17 +28,16 @@ If applicable, add screenshots to help explain your problem.

**Crash Report**

If the bug resulted in an error message, please click on "Download Error
Message" and attach it here

Please, also fill out the following information manually:
If the bug resulted in an error message, please click on "Download Error Message" and attach it here. If there is no error, please follow [these instructions](https://discourse.specifysoftware.org/t/download-specify-7-system-information/1614) to download your System Information.

Please fill out the following information manually:
- OS: [e.g. Windows 10]
- Browser: [e.g. Chrome, Safari]
- Specify 7 Version: [e.g. 7.6.1]
- Database Name: [e.g. kufish] (you can see this in the "About Specify 7" dialog)
- Collection name: [e.g. KU Fish Tissue]
- User Name: [e.g. SpAdmin]
- URL: [e.g. https://ku_fish-v7.test.specifysystems.org/specify/workbench/215/]

**Reported By**
Name of your institution
Expand Down
50 changes: 45 additions & 5 deletions specifyweb/businessrules/migrations/0002_default_unique_rules.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,53 @@
from django.db import migrations

from specifyweb.specify import models as spmodels
from specifyweb.businessrules.uniqueness_rules import apply_default_uniqueness_rules
from specifyweb.specify.datamodel import datamodel
from specifyweb.businessrules.uniqueness_rules import apply_default_uniqueness_rules, rule_is_global, DEFAULT_UNIQUENESS_RULES


def apply_rules_to_discipline(apps, schema_editor):
for disp in spmodels.Discipline.objects.all():
def apply_default_rules(apps, schema_editor):
Discipline = apps.get_model('specify', 'Discipline')
for disp in Discipline.objects.all():
apply_default_uniqueness_rules(disp)


def remove_default_rules(apps, schema_editor):
Discipline = apps.get_model('specify', 'Discipline')
UniquenessRule = apps.get_model('businessrules', 'UniquenessRule')
UniquenessRuleFields = apps.get_model(
'businessrules', 'UniquenessRuleField')

for discipline in Discipline.objects.all():
remove_rules_from_discipline(
discipline, UniquenessRule, UniquenessRuleFields)


def remove_rules_from_discipline(discipline, uniqueness_rule, uniquenessrule_fields):
for table, rules in DEFAULT_UNIQUENESS_RULES.items():
model_name = datamodel.get_table_strict(table).django_name
for rule in rules:
to_remove = set()
fields, scopes = rule["rule"]
isDatabaseConstraint = rule["isDatabaseConstraint"]

is_global = rule_is_global(scopes)

for field in fields:
found_fields = uniquenessrule_fields.objects.filter(uniquenessrule__modelName=model_name, uniquenessrule__isDatabaseConstraint=isDatabaseConstraint,
uniquenessrule__discipline_id=None if is_global else discipline.id, fieldPath=field, isScope=False)

to_remove.update(
tuple(found_fields.values_list('uniquenessrule_id', flat=True)))
found_fields.delete()
for scope in scopes:
found_scopes = uniquenessrule_fields.objects.filter(uniquenessrule__modelName=model_name, uniquenessrule__isDatabaseConstraint=isDatabaseConstraint,
uniquenessrule__discipline_id=None if is_global else discipline.id, fieldPath=scope, isScope=True)

to_remove.update(
tuple(found_scopes.values_list('uniquenessrule_id', flat=True)))
found_scopes.delete()
uniqueness_rule.objects.filter(id__in=tuple(to_remove)).delete()


class Migration(migrations.Migration):
initial = True

Expand All @@ -18,5 +57,6 @@ class Migration(migrations.Migration):
]

operations = [
migrations.RunPython(apply_rules_to_discipline),
migrations.RunPython(apply_default_rules,
remove_default_rules, atomic=True),
]
5 changes: 3 additions & 2 deletions specifyweb/businessrules/rules/cogtype_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
from specifyweb.businessrules.orm_signal_handler import orm_signal_handler
from specifyweb.specify.models import Picklist, Picklistitem

SYSTEM_COGTYPES_PICKLIST = "SystemCOGTypes"

@orm_signal_handler('pre_save', 'Collectionobjectgrouptype')
def cogtype_pre_save(cog_type):

# Ensure the cog_type type is validated by being the picklist.
# NOTE: Maybe add constraint on the cog_type name in the future.
default_cog_types_picklist = Picklist.objects.get(
name="Default Collection Object Group Types",
tablename="collectionobjectgrouptype",
name=SYSTEM_COGTYPES_PICKLIST,
collection=cog_type.collection
)
if Picklistitem.objects.filter(picklist=default_cog_types_picklist, value=cog_type.type).count() == 0:
Expand Down
64 changes: 50 additions & 14 deletions specifyweb/businessrules/uniqueness_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
from django.db import connections
from django.db.migrations.recorder import MigrationRecorder
from django.core.exceptions import ObjectDoesNotExist
from specifyweb.specify import models
from specifyweb.specify.api import get_model
from specifyweb.specify.datamodel import datamodel
from specifyweb.middleware.general import serialize_django_obj
from specifyweb.specify.scoping import in_same_scope
from .orm_signal_handler import orm_signal_handler
from .exceptions import BusinessRuleException
from .models import UniquenessRule
from . import models

DEFAULT_UNIQUENESS_RULES: Dict[str, List[Dict[str, Union[List[List[str]], bool]]]] = json.load(
open('specifyweb/businessrules/uniqueness_rules.json'))
Expand All @@ -29,7 +29,13 @@
@orm_signal_handler('pre_save', None, dispatch_uid=UNIQUENESS_DISPATCH_UID)
def check_unique(model, instance):
model_name = instance.__class__.__name__
rules = UniquenessRule.objects.filter(modelName=model_name)
cannonical_model = get_model(model_name)

if not cannonical_model:
# The model is not a Specify Model
# probably a Django-specific model
return

applied_migrations = MigrationRecorder(
connections['default']).applied_migrations()

Expand All @@ -40,14 +46,23 @@ def check_unique(model, instance):
else:
return

# We can't directly use the main app registry in the context of migrations, which uses fake models
registry = model._meta.apps

UniquenessRule = registry.get_model('businessrules', 'UniquenessRule')
UniquenessRuleField = registry.get_model(
'businessrules', 'UniquenessRuleField')

rules = UniquenessRule.objects.filter(modelName=model_name)
for rule in rules:
if not rule_is_global(tuple(field.fieldPath for field in rule.fields.filter(isScope=True))) and not in_same_scope(rule, instance):
rule_fields = UniquenessRuleField.objects.filter(uniquenessrule=rule)
if not rule_is_global(tuple(field.fieldPath for field in rule_fields.filter(isScope=True))) and not in_same_scope(rule, instance):
continue

field_names = [
field.fieldPath.lower() for field in rule.fields.filter(isScope=False)]
field.fieldPath.lower() for field in rule_fields.filter(isScope=False)]

_scope = rule.fields.filter(isScope=True)
_scope = rule_fields.filter(isScope=True)
scope = None if len(_scope) == 0 else _scope[0]

all_fields = [*field_names]
Expand Down Expand Up @@ -138,7 +153,9 @@ def join_with_and(fields):
return ' and '.join(fields)


def apply_default_uniqueness_rules(discipline: models.Discipline):
def apply_default_uniqueness_rules(discipline, registry=None):
UniquenessRule = registry.get_model(
'businessrules', 'UniquenessRule') if registry else models.UniquenessRule
has_set_global_rules = len(
UniquenessRule.objects.filter(discipline=None)) > 0

Expand All @@ -156,15 +173,34 @@ def apply_default_uniqueness_rules(discipline: models.Discipline):
_discipline = None

create_uniqueness_rule(
model_name, _discipline, isDatabaseConstraint, fields, scopes)
model_name, _discipline, isDatabaseConstraint, fields, scopes, registry)


def create_uniqueness_rule(model_name, discipline, is_database_constraint, fields, scopes, registry=None):
UniquenessRule = registry.get_model(
'businessrules', 'UniquenessRule') if registry else models.UniquenessRule
UniquenessRuleField = registry.get_model(
'businessrules', 'UniquenessRuleField') if registry else models.UniquenessRuleField

matching_fields = UniquenessRuleField.objects.filter(
fieldPath__in=fields, uniquenessrule__modelName=model_name, uniquenessrule__isDatabaseConstraint=is_database_constraint, uniquenessrule__discipline=discipline, isScope=False)

matching_scopes = UniquenessRuleField.objects.filter(
fieldPath__in=scopes, uniquenessrule__modelName=model_name, uniquenessrule__isDatabaseConstraint=is_database_constraint, uniquenessrule__discipline=discipline, isScope=True)

# If the rule already exists, skip creating the rule
if len(matching_fields) == len(fields) and len(matching_scopes) == len(scopes):
return

rule = UniquenessRule.objects.create(
discipline=discipline, modelName=model_name, isDatabaseConstraint=is_database_constraint)

def create_uniqueness_rule(model_name, discipline, is_database_constraint, fields, scopes) -> UniquenessRule:
created_rule = UniquenessRule.objects.create(discipline=discipline,
modelName=model_name, isDatabaseConstraint=is_database_constraint)
created_rule.fields.set(fields)
created_rule.fields.add(
*scopes, through_defaults={"isScope": True})
for field in fields:
UniquenessRuleField.objects.create(
uniquenessrule=rule, fieldPath=field, isScope=False)
for scope in scopes:
UniquenessRuleField.objects.create(
uniquenessrule=rule, fieldPath=scope, isScope=True)


"""If a uniqueness rule has a scope which traverses through a hiearchy
Expand Down
4 changes: 2 additions & 2 deletions specifyweb/context/schema_localization.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def get_schema_localization(collection, schematype, lang):
cfields = ('format', 'ishidden', 'isuiformatter', 'picklistname', 'type', 'aggregator', 'defaultui', 'name', 'desc')

containers = {
row[0]: dict(items={}, **{field: row[i+1] for i, field in enumerate(cfields)})
row[0].lower(): dict(items={}, **{field: row[i+1] for i, field in enumerate(cfields)})
for row in cursor.fetchall()
}

Expand Down Expand Up @@ -172,7 +172,7 @@ def get_schema_localization(collection, schematype, lang):
ifields = ('format', 'ishidden', 'isuiformatter', 'picklistname', 'type', 'isrequired', 'weblinkname', 'name', 'desc')

for row in cursor.fetchall():
containers[row[0]]['items'][row[1].lower()] = {field: row[i+2] for i, field in enumerate(ifields)}
containers[row[0].lower()]['items'][row[1].lower()] = {field: row[i+2] for i, field in enumerate(ifields)}

return containers

4 changes: 3 additions & 1 deletion specifyweb/context/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from specifyweb.specify.models import Collection, Institution, \
Specifyuser, Spprincipal, Spversion
from specifyweb.specify.schema import base_schema
from specifyweb.specify.api import uri_for_model
from specifyweb.specify.serialize_datamodel import datamodel_to_json
from specifyweb.specify.specify_jar import specify_jar
from specifyweb.specify.views import login_maybe_required, openapi
Expand Down Expand Up @@ -340,7 +341,8 @@ def domain(request):
'embeddedPaleoContext': collection.discipline.ispaleocontextembedded,
'paleoContextChildTable': collection.discipline.paleocontextchildtable,
'catalogNumFormatName': collection.catalognumformatname,
}
'defaultCollectionObjectType': uri_for_model(collection.collectionobjecttype.__class__, collection.collectionobjecttype.id) if collection.collectionobjecttype is not None else None
}

return HttpResponse(json.dumps(domain), content_type='application/json')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`fields are loaded 1`] = `
[
"[literalField CollectionObject.actualTotalCountAmt]",
"[literalField CollectionObject.age]",
"[literalField CollectionObject.availability]",
"[literalField CollectionObject.catalogNumber]",
"[literalField CollectionObject.catalogedDate]",
Expand Down Expand Up @@ -104,6 +105,7 @@ exports[`fields are loaded 1`] = `
exports[`literal fields are loaded 1`] = `
[
"[literalField CollectionObject.actualTotalCountAmt]",
"[literalField CollectionObject.age]",
"[literalField CollectionObject.availability]",
"[literalField CollectionObject.catalogNumber]",
"[literalField CollectionObject.catalogedDate]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ describe('Collection Object business rules', () => {
const getBaseCollectionObject = () =>
new tables.CollectionObject.Resource({
id: collectionObjectlId,
collectionobjecttype: collectionObjectTypeUrl,
determinations: [
{
taxon: getResourceApiUrl('Taxon', otherTaxonId),
Expand Down Expand Up @@ -116,6 +115,9 @@ describe('Collection Object business rules', () => {
const collectionObject = getBaseCollectionObject();

expect(collectionObject.get('collectingEvent')).toBeDefined();
expect(collectionObject.get('collectionObjectType')).toEqual(
schema.defaultCollectionObjectType
);
});

const otherCollectionObjectTypeUrl = getResourceApiUrl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ test('getFieldsToNotClone', () => {
});
expect(getFieldsToNotClone(tables.CollectionObject, true, false)).toEqual([
'actualTotalCountAmt',
'age',
'catalogNumber',
'timestampModified',
'guid',
Expand All @@ -304,6 +305,7 @@ test('getFieldsToNotClone', () => {
]);
expect(getFieldsToNotClone(tables.CollectionObject, false, false)).toEqual([
'actualTotalCountAmt',
'age',
'catalogNumber',
'timestampModified',
'guid',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const determinationsResponse: RA<Partial<SerializedRecord<Determination>>> = [

const collectionObjectResponse = {
id: collectionObjectId,
collectionobjecttype: getResourceApiUrl('CollectionObjectType', 1),
resource_uri: collectionObjectUrl,
accession: accessionUrl,
catalognumber: '000029432',
Expand Down Expand Up @@ -203,6 +204,7 @@ describe('needsSaved', () => {
const resource = new tables.CollectionObject.Resource({
id: collectionObjectId,
});

expect(resource.needsSaved).toBe(false);
resource.set('text1', 'a');
expect(resource.needsSaved).toBe(true);
Expand All @@ -212,6 +214,7 @@ describe('needsSaved', () => {
const resource = new tables.CollectionObject.Resource({
id: collectionObjectId,
});

expect(resource.needsSaved).toBe(false);
resource.set('determinations', []);
expect(resource.needsSaved).toBe(true);
Expand Down Expand Up @@ -329,7 +332,7 @@ describe('placeInSameHierarchy', () => {

test('invalid hierarchy', async () => {
const collectionObject = new tables.CollectionObject.Resource({
id: 100,
id: collectionObjectId,
});
const author = new tables.Author.Resource();
await expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ test('domain data is fetched and parsed correctly', async () =>
},
embeddedCollectingEvent: false,
embeddedPaleoContext: true,
defaultCollectionObjectType: '/api/specify/collectionobjecttype/1/',
fieldPartSeparator: '-',
orgHierarchy: [
'CollectionObject',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ test('indexed fields are loaded', () =>
{
"accession": "[relationship CollectionObject.accession]",
"actualTotalCountAmt": "[literalField CollectionObject.actualTotalCountAmt]",
"age": "[literalField CollectionObject.age]",
"agent1": "[relationship CollectionObject.agent1]",
"altCatalogNumber": "[literalField CollectionObject.altCatalogNumber]",
"appraisal": "[relationship CollectionObject.appraisal]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import type { SpecifyResource } from './legacyTypes';
import { fetchResource, idFromUrl } from './resource';
import { setSaveBlockers } from './saveBlockers';
import { schema } from './schema';
import type { Collection } from './specifyTable';
import { tables } from './tables';
import type {
Expand Down Expand Up @@ -154,6 +155,16 @@ export const businessRuleDefs: MappedBusinessRuleDefs = {
new tables.CollectingEvent.Resource()
);
}

// Set the default CoType
if (
typeof schema.defaultCollectionObjectType === 'string' &&
typeof collectionObject.get('collectionObjectType') !== 'string'
)
collectionObject.set(
'collectionObjectType',
schema.defaultCollectionObjectType
);
},
fieldChecks: {
collectionObjectType: async (resource): Promise<undefined> => {
Expand Down
Loading

0 comments on commit 5d547c6

Please sign in to comment.