Fully Custom Location#650
Conversation
|
[diff-counting] Significant lines: 535. |
mjaydenkim
left a comment
There was a problem hiding this comment.
Small note: I did notice that there are some overlaps here and #647; I'll sort those out when I have time.
Everything works cleanly and the map enabling/disabling based on custom locations is quite nice, better than I had. I think worrying about duplicate locations is a next semester issue imo, probably shouldn't raise major issues during winter testing. Didn't do a super robust review so another one would be great but I didn't notice any problems, this looks pretty seamless to me!! Great work!
YottaYocta
left a comment
There was a problem hiding this comment.
I second @mjaydenkim; these changes look good! @anika-4444 wanted your opinion but should we add a way for the admins to request a custom ride in this PR, or do so in a future one? I noticed that admins could only add rides based on existing locations.
|
@YottaYocta i believe that’s in my pr #647 which is kinda outdated after this one but i can merge that element in! |
|
That would be very cool |
91dada0 to
9871c82
Compare
Summary
This pull request is the first step towards implementing feature Foo
Notes
Because custom locations no longer require an address, the Google Maps got kind of wonky. To work around this problem, I added a placeholder for the map that appears when a custom location is used.
Overall, this method could cause more repeat locations than what was there before (which only created new locations based on if the latitude/longitude could not be found in the db), but because we cannot require a real address, I think we will have to just deal with this. One way to mitigate this could be to search the db for locations that have the same name. Will add this asap