RefugeRestrooms / refugerestrooms

REFUGE restrooms indexes and maps safe restroom locations for trans, intersex, and gender nonconforming individuals.
http://www.refugerestrooms.org
GNU Affero General Public License v3.0
894 stars 261 forks source link

db/seeds.rb: Give restroom entries an edit_id #567

Closed DeeDeeG closed 5 years ago

DeeDeeG commented 5 years ago

Context

Summary of Changes

In db/seeds.rb:

[Edited the above: This no-longer count up the edit_id number from 1 by iterating the loop; It now reads the id value of the restroom entry, and copies that as the (matching) edit_id value.]

Checklist

Screenshots

Before

Refuge Restrooms API documentation page, showing only one restroom entry with edit_id: 0

After

API documentation page, showing multiple restroom entries, where each entry has matching edit_id and id values

mi-wood commented 5 years ago

I think it might be better to do something similar to how the application creates restrooms. So:

restroom = Restroom.create(...)
restroom.update(edit_id: restroom.id)

That way it is tied to the actual id and not the index of the for loop.

DeeDeeG commented 5 years ago

Thanks for the code suggestion, looks good.

I got kind of confused trying to add that in, because typing the wrong name (restroom vs Restroom) did completely different things. For clarity, would you mind if I did this:

current_restroom = Restroom.create(...)
current_restroom.update(edit_id: current_restroom.id)

Or something similar?

(Edit to add: I admit I'm not much of a programmer, so this may not be a problem to folks who program more often. Now that I "get" that lowercase restroom is just a temporary "object," and its name doesn't matter, and Restroom is a... "function" (?) defined for the project, I can see why they behave differently. So it may not be necessary to use names that are more distinct at a glance. But it seems really easy to typo these, even for more-experienced programmers. Since I don't mess with the code a lot, I defer to folks who do.)

(Edit 2: Regardless of what name we use for the local/temporary restroom object, I can confirm the code as you posted it works 100%.)

mi-wood commented 5 years ago

So, Restroom is a class name.

Restroom.create is a class method. This is a method is not associated with a single restroom, hence it is used for create (the restroom doesn't exist yet). It returns an instance of Restroom.

restroom.update is an instance method. It acts upon a restroom. In this case, update is acting upon the existing restroom to get its id and update the edit_id.

class_name = ClassName.new() is a pretty common convention. That being said, I'm not too particular about what it's called.

DeeDeeG commented 5 years ago

I went with just lowercase restroom for the instance name, because, as you say, it is a common pattern in Ruby, and people editing this file are likely to be comfortable with common Ruby patterns.

Going with the flow is fine here IMO.

(This can be squash-and-merged for neatness' sake in develop/master.)

DeeDeeG commented 5 years ago

I'm going to merge this soon if that's okay/no objections. (Definitely want to squash&merge though.)