Skip to content
This repository has been archived by the owner on Feb 2, 2022. It is now read-only.

Commit

Permalink
Fix update report model function (#237)
Browse files Browse the repository at this point in the history
The update function should not update the model with empty fatalities.
  • Loading branch information
rgreinho committed Feb 22, 2020
1 parent d6fe076 commit 18fd73e
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 6 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [[3.0.5]] - 2020-02-22

### Fixed

- Fix `model.Report` update function. The model won't be updated with empty fatalities. [#237]

## [[3.0.4]] - 2020-02-15

### Fixed
Expand Down Expand Up @@ -202,6 +208,7 @@ This first version allows a user to retrieve traffic fatality repports for a cer
[3.0.2]: https://github.com/scrapd/scrapd/releases/3.0.2
[3.0.3]: https://github.com/scrapd/scrapd/releases/3.0.3
[3.0.4]: https://github.com/scrapd/scrapd/releases/3.0.4
[3.0.5]: https://github.com/scrapd/scrapd/releases/3.0.5

[//]: # (Issue/PR links)
[#13]: https://github.com/scrapd/scrapd/issues/13
Expand Down Expand Up @@ -255,3 +262,4 @@ This first version allows a user to retrieve traffic fatality repports for a cer
[#227]: https://github.com/scrapd/scrapd/pull/227
[#230]: https://github.com/scrapd/scrapd/pull/230
[#234]: https://github.com/scrapd/scrapd/pull/234
[#237]: https://github.com/scrapd/scrapd/pull/237
12 changes: 7 additions & 5 deletions scrapd/core/model.py
Expand Up @@ -114,21 +114,23 @@ def update(self, other, strict=False):

# When strict...
if strict:
# The case number and date must be identical.
# The case number must be identical.
if not all([getattr(self, attr) == getattr(other, attr) for attr in required_attrs]):
raise ValueError(
f'in strict mode the required attributes "({", ".join(required_attrs)})" must be identical')
else:
# Otherwise they are overridden.
# Otherwise the required attributes are overridden.
for attr in required_attrs:
setattr(self, attr, getattr(other, attr))

# Set the non-empty attributes of `other` into the empty attributes of the current instance.
for attr in attrs:
# Fatalities are a special case because we cannot simply update their attributes individually.
# Therefore it is all or nothing, but we want to make sure we do not update it with empty values.
if attr == 'fatalities':
setattr(self, attr, getattr(other, attr))
continue
if not getattr(self, attr) and getattr(other, attr):
if getattr(other, attr):
setattr(self, attr, getattr(other, attr))
elif not getattr(self, attr) and getattr(other, attr):
setattr(self, attr, getattr(other, attr))

@validator('case')
Expand Down
41 changes: 40 additions & 1 deletion tests/core/test_model.py
Expand Up @@ -166,6 +166,44 @@
'strict': False,
'id': 'update-multi-fatalities',
},
{
'input_': model.Report(
case='19-123456',
date=now.date(),
link='link',
fatalities=[
model.Fatality(
age=21,
dob=datetime.date(1998, 4, 28),
ethnicity=model.Ethnicity.white,
first='Owen',
gender=model.Gender.male,
last='Macki',
middle='William',
),
],
),
'other': model.Report(case='19-123456', date=now.date(), link='other link', crash=1, fatalities=[]),
'expected': model.Report(
case='19-123456',
date=now.date(),
link='link',
crash=1,
fatalities=[
model.Fatality(
age=21,
dob=datetime.date(1998, 4, 28),
ethnicity=model.Ethnicity.white,
first='Owen',
gender=model.Gender.male,
last='Macki',
middle='William',
),
],
),
'strict': True,
'id': 'update-no-fatalities',
},
]


Expand Down Expand Up @@ -252,7 +290,8 @@ def test_update_00(self, input_, other, strict, expected):
"""Ensure models can be updated."""
actual = input_.copy(deep=True)
actual.update(other, strict)
assert actual == expected
# assert actual == expected
assert actual.dict() == expected.dict()

def test_update_01(self):
"""Ensure required fields are identical in strict mode."""
Expand Down

0 comments on commit 18fd73e

Please sign in to comment.