Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field changes to the territorial entity. #127

Merged
merged 12 commits into from
Jun 26, 2019
Merged

Field changes to the territorial entity. #127

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
78e5e0b
Added inception and dissolution dates for terretorial entities
ataalik May 11, 2019
728a236
Added label to TE
ataalik May 11, 2019
c54418e
made dates emptry and nullable
ataalik May 21, 2019
9097cc4
Added stv count to TE
ataalik Jun 8, 2019
0a37bfb
Merge branch 'ata-99' of github.com:chronhq/backend into ata-99
ataalik Jun 8, 2019
0c26622
Lint and simplify migrations
ataalik Jun 8, 2019
88e5fc7
Moved stv_count to views
ataalik Jun 11, 2019
b3ae534
Made inception_date nullable
ataalik Jun 13, 2019
d0fb730
Split tests across two files
ataalik Jun 13, 2019
1a1d03e
Added new pylintrc for tests module
ataalik Jun 26, 2019
1635881
Update travis-ci test config
MiklerGM Jun 26, 2019
194202d
Ignore non-json response from wikidata
MiklerGM Jun 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions project/api/migrations/0011_auto_20190511_1557.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Generated by Django 2.2.1 on 2019-05-11 15:57

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('api', '0010_auto_20190430_0334'),
]

operations = [
migrations.AddField(
model_name='historicalterritorialentity',
name='dissolution_date',
field=models.DecimalField(decimal_places=1, default=0, max_digits=10),
preserve_default=False,
),
migrations.AddField(
model_name='historicalterritorialentity',
name='inception_date',
field=models.DecimalField(decimal_places=1, default=0, max_digits=10),
preserve_default=False,
),
migrations.AddField(
model_name='territorialentity',
name='dissolution_date',
field=models.DecimalField(decimal_places=1, default=0, max_digits=10),
preserve_default=False,
),
migrations.AddField(
model_name='territorialentity',
name='inception_date',
field=models.DecimalField(decimal_places=1, default=0, max_digits=10),
preserve_default=False,
),
]
25 changes: 25 additions & 0 deletions project/api/migrations/0012_auto_20190511_1812.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django 2.2.1 on 2019-05-11 18:12

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('api', '0011_auto_20190511_1557'),
]

operations = [
migrations.AddField(
model_name='historicalterritorialentity',
name='label',
field=models.TextField(default='default', max_length=90),
preserve_default=False,
),
migrations.AddField(
model_name='territorialentity',
name='label',
field=models.TextField(default='default', max_length=90),
preserve_default=False,
),
]
38 changes: 38 additions & 0 deletions project/api/migrations/0013_auto_20190608_0503.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Generated by Django 2.2.1 on 2019-06-08 05:03

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('api', '0012_auto_20190511_1812'),
]

operations = [
migrations.AddField(
model_name='historicalterritorialentity',
name='stv_count',
field=models.IntegerField(blank=True, default=0, null=True),
),
migrations.AddField(
model_name='territorialentity',
name='predecessor',
field=models.ManyToManyField(blank=True, to='api.TerritorialEntity'),
),
migrations.AddField(
model_name='territorialentity',
name='stv_count',
field=models.IntegerField(blank=True, default=0, null=True),
),
migrations.AlterField(
model_name='historicalterritorialentity',
name='dissolution_date',
field=models.DecimalField(blank=True, decimal_places=1, max_digits=10, null=True),
),
migrations.AlterField(
model_name='territorialentity',
name='dissolution_date',
field=models.DecimalField(blank=True, decimal_places=1, max_digits=10, null=True),
),
]
31 changes: 31 additions & 0 deletions project/api/migrations/0014_auto_20190613_0622.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 2.2.1 on 2019-06-13 06:22

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('api', '0013_auto_20190608_0503'),
]

operations = [
migrations.RemoveField(
model_name='historicalterritorialentity',
name='stv_count',
),
migrations.RemoveField(
model_name='territorialentity',
name='stv_count',
),
migrations.AlterField(
model_name='historicalterritorialentity',
name='inception_date',
field=models.DecimalField(blank=True, decimal_places=1, max_digits=10, null=True),
),
migrations.AlterField(
model_name='territorialentity',
name='inception_date',
field=models.DecimalField(blank=True, decimal_places=1, max_digits=10, null=True),
),
]
19 changes: 19 additions & 0 deletions project/api/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,18 @@ class TerritorialEntity(models.Model):
"""

wikidata_id = models.PositiveIntegerField() # Excluding the Q
label = models.TextField(max_length=90)
Copy link
Member

@quorth0n quorth0n May 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a nullable PointField similar to visual_center, and updated as a post create hook

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, on second thought we should probably keep this field for #21. We will still need the PointField though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have Point Field for the visual center in STV's table

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my comment was before the Slack post. Originally I was thinking a field here could correspond to P625 for a wikdiata entity but it doesn't seem that property does us much good in any situation.

color = ColorField()
admin_level = models.PositiveIntegerField()
inception_date = models.DecimalField(
decimal_places=1, max_digits=10, blank=True, null=True
)
dissolution_date = models.DecimalField(
decimal_places=1, max_digits=10, blank=True, null=True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that we should do this strict validation on the inception date?

Especially when you are validating it for not null

backend/project/api/models.py

Lines 55 to 60 in 88e5fc7

def clean(self, *args, **kwargs): # pylint: disable=W0221
if not self.inception_date is None and not self.dissolution_date is None:
if self.inception_date > self.dissolution_date:
raise ValidationError(
"Inception date cannot be later than dissolution date"
) 

postgres=# INSERT INTO api_territorialentity VALUES (1, 30, '10', 2, NULL, NULL, 'USA');
ERROR:  null value in column "inception_date" violates not-null constraint
DETAIL:  Failing row contains (1, 30, 10, 2, null, null, USA).
postgres=# INSERT INTO api_territorialentity (wikidata_id, color, admin_level, label) VALUES (30, '10', 2, 'USA');
ERROR:  null value in column "inception_date" violates not-null constraint
DETAIL:  Failing row contains (1, 30, 10, 2, null, null, USA).

postgres=# INSERT INTO api_territorialentity VALUES (1, 30, '10', 2, 0, 0, 'USA');
INSERT 0 1
postgres=# INSERT INTO api_territorialentity VALUES (2, 30, '10', 2, NULL, 0, 'USA2');
INSERT 0 1
postgres=# select * from api_territorialentity;
 id | wikidata_id | color | admin_level | dissolution_date | inception_date | label
----+-------------+-------+-------------+------------------+----------------+-------
  1 |          30 | 10    |           2 |              0.0 |            0.0 | USA
  2 |          30 | 10    |           2 |                  |            0.0 | USA2
(2 rows)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question, why inception and dissolution date are returned as a string via API request?

{
  "id":2,
  "stvs":[],
  "stv_count":0,
  "wikidata_id":30,
  "label":"USA2",
  "color":"10",
  "admin_level":2,
  "inception_date":"0.0",
  "dissolution_date":null,
  "predecessor":[],
  "relations":[]
}

I've noticed that we have one more reserved SQL word as a column - references, #119

The response after adding an STV

    {
        "id": 1,
        "stvs": [ // This is an array of all stvs
            {
                "id": 1,
                "start_date": "1.0",
                "end_date": "0.0",
                "references": [
                    "qwe",
                    "qwee"
                ],
                "visual_center": {
                    "type": "Point",
                    "coordinates": [
                        -71.060316,
                        48.432044
                    ]
                },
                "entity": 1,
                "related_events": []
            }
        ],
        "stv_count": 1, // this is a new count field
        "wikidata_id": 30,
        "label": "USA",
        "color": "10",
        "admin_level": 2,
        "inception_date": "0.0",
        "dissolution_date": "0.0",
        "predecessor": [],
        "relations": []
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking thinking the dates as a user input but of course since we have gaps in wikidata both date fields should have less restriction. I guess they should be nullable?

I need to convert the dates using the library in the viewset I think. Will take care of it.

What should references be changed as. Citations comes to mind.

)

predecessor = models.ManyToManyField("self", blank=True, symmetrical=False)

relations = models.ManyToManyField(
"self",
blank=True,
Expand All @@ -44,6 +54,15 @@ class TerritorialEntity(models.Model):
)
history = HistoricalRecords()

def clean(self, *args, **kwargs): # pylint: disable=W0221
if not self.inception_date is None and not self.dissolution_date is None:
if self.inception_date > self.dissolution_date:
raise ValidationError(
"Inception date cannot be later than dissolution date"
)

super(TerritorialEntity, self).clean(*args, **kwargs)

def get_children(self):
"""
Returns relations in which this nation is a parent
Expand Down
1 change: 1 addition & 0 deletions project/api/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class TerritorialEntitySerializer(ModelSerializer):
"""

stvs = SpacetimeVolumeSerializerNoTerritory(many=True, read_only=True)
stv_count = IntegerField(read_only=True)

class Meta:
model = TerritorialEntity
Expand Down
2 changes: 2 additions & 0 deletions project/api/tests/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from .model_tests import *
from .api_tests import *
Loading