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

Data from db/export.csv should get "edit_id" values matching their "id" values (currently all set to zero) #541

Closed
DeeDeeG opened this issue Dec 4, 2018 · 4 comments
Labels

Comments

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 4, 2018

Scope / difficulty of fixing this

Not too hard I hope?

Impact

Only affects local testing (in Docker, Vagrant, etc.) with the db/export.csv data. Fixing this will make the data from db/export.csv behave properly alongside the new user edit proposals system.

This has no bearing on our production instance, because the data there all have properedit_id values.

Problem

Data in db/export.csv have no actual edit_id values to begin with.

Rather than being migrated (or otherwise automatically processed) to have an edit_id matching their id, they all end up having edit_id: 0.

Since we now group API results by edit_id and only display one result per edit_id, the app only ever displays at most one result regardless of the specifics of the API call. (This will tend to be the Dolores Park Cafe entry, since it has the highest id value).

(From the documentary about software development, Highlander: "There can be only one!")

How to reproduce the problem

  • Run the app locally (for example in Docker or Vagrant)
  • Search for restrooms near "San Francisco" anywhere in the app
    • or visit the API documentation page and make any API call
  • Observe there are either no relevant results (not related to this bug) or else only one result (because of this bug).

Proposed solution to this bug

Process the db/export.csv data in some way (a migration? somewhere else in our Ruby code?) so they have edit_id values matching their id values.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Dec 4, 2018

I'm also unsure if we should even be grouping by edit_id.

I think that value should always be unique, and we should group by id instead?

If I understand it correctly:

  1. A new restroom is added. Its id and edit_id match
  2. A user proposes a change and we accept it
  3. The changed version of the restroom has the original id but a fresh edit_id

Thus no two entries have a shared edit_id, and grouping by id produces only the freshest edit of a given restroom entry.

Might be wrong about this, would love some comment from @mi-wood and/or @stardust66.

@stardust66
Copy link
Contributor

stardust66 commented Mar 23, 2019

From the way I understand the editing process, when an edit is submitted, a new restroom entry is made with a new id and an edit_id matching the id of the original restroom. Thus, grouping by edit_id ensures that the newest restroom, which is the one with the highest id, gets displayed.

I'm taking a look at db/export.csv and the problem seems to be that there's no edit_id in the csv at all. I think I'll just change db/seeds.rb so that it adds edit_ids to each restroom entry when loading the csv.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Mar 24, 2019

We could also manually edit export.csv instead, since it's only 21 entries long. I'd be able to do that without it taking too long.

Edit: Whoops, I guess not. There are no field names in the db/export.csv, only values. So the id must be getting set implicitly, in order, based on which entry is created first (id => 1), second (id => 2).

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Mar 25, 2019

I have a branch where I seem to have fixed this: DeeDeeG@27d3e15

Based on this stackoverflow answer: https://stackoverflow.com/a/12408934

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants