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.

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

Ignore updated_at and updated_by field values when updating plots via the API. #2963

Merged

Conversation

RobinIsTheBird
Copy link
Contributor

The android app sends updated_by. It doesn't know it's a foreign key,
neither does the api, so it gets set to the id rather than the model.
This causes a ValueError in auditing.

The api already has a list of attributes to ignore.
updated_by can be safely ignored because it is set in auditing.
Same for updated_at. Since they are supposed to be set in auditing,
they really should both be ignored.

So ignore them.

--

Connects to OpenTreeMap/otm-android#303
Connects to OpenTreeMap/otm-android#304

The android app sends `updated_by`. It doesn't know it's a foreign key,
neither does the api, so it gets set to the `id` rather than the model.
This causes a `ValueError` in auditing.

The api already has a list of attributes to ignore.
`updated_by` can be safely ignored because it is set in auditing.
Same for `updated_at`. Since they are supposed to be set in auditing,
they really should both be ignored.

So ignore them.

--

Connects to OpenTreeMap/otm-android#303
Connects to OpenTreeMap/otm-android#304
@RobinIsTheBird
Copy link
Contributor Author

It passed jenkins when mis-targeted at develop. No reason to expect it to fail now, so assigning it.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 83.286% when pulling 78766cd on RobinIsTheBird:rtb/fix-edit into 1a811cc on OpenTreeMap:release/2.14.0.

@jwalgran jwalgran self-assigned this Jan 30, 2017
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

I reproduced OpenTreeMap/otm-android#303 and OpenTreeMap/otm-android#304 and verified that this prevents them.

We do always want the server to be the authority on updated_by and updated_at.

@jwalgran jwalgran assigned RobinIsTheBird and unassigned jwalgran Jan 30, 2017
@jwalgran
Copy link
Contributor

I would like to change the title of this PR to the more descriptive summary "Ignore updated_at and updated_by field values when updating plots via the API."

@RobinIsTheBird RobinIsTheBird changed the title Fix api edit Ignore updated_at and updated_by field values when updating plots via the API. Jan 31, 2017
@RobinIsTheBird RobinIsTheBird merged commit 9a433df into OpenTreeMap:release/2.14.0 Jan 31, 2017
@RobinIsTheBird RobinIsTheBird deleted the rtb/fix-edit branch January 31, 2017 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants