MAPC / massbuilds-legacy

Crowdsourcing authoritative info on recent & ongoing developments in Massachusetts.
3 stars 5 forks source link

Fix #148 by reverting "Check if URLs exist" #170

Closed mzagaja closed 7 years ago

mzagaja commented 7 years ago

This reverts commit f5ca1d7a19a6621213aa755bab574a5d69c873d7. This is being done because URL validations were not accounting for variances in how humans were typing URLs and thus failing improperly.

mzagaja commented 7 years ago

I wrongly assumed I wouldn't need to test this revert but when I went to test something else after I made this commit I got the following error:

~/G/massbuilds ❯❯❯ foreman start                                                                                                                    mjz/148-remove-url-validations ✭
13:45:34 web.1  | started with pid 31190
13:45:35 web.1  | [31190] Puma starting in cluster mode...
13:45:35 web.1  | [31190] * Version 3.4.0 (ruby 2.3.1-p112), codename: Owl Bowl Brawl
13:45:35 web.1  | [31190] * Min threads: 5, max threads: 5
13:45:35 web.1  | [31190] * Environment: development
13:45:35 web.1  | [31190] * Process workers: 2
13:45:35 web.1  | [31190] * Preloading application
13:45:42 web.1  | [31190] ! Unable to load application: LoadError: cannot load such file -- /Users/mzagaja/GitHub/massbuilds/lib/extensions/active_record
13:45:42 web.1  | /Users/mzagaja/.rvm/gems/ruby-2.3.1@dd/gems/activesupport-4.2.5.1/lib/active_support/dependencies.rb:274:in `require': cannot load such file -- /Users/mzagaja/GitHub/massbuilds/lib/extensions/active_record (LoadError)
allthesignals commented 7 years ago

@mzagaja I tried this locally, and submitting an edit throws this error:

NoMethodError at /developments/1/edit
undefined method `[]' for nil:NilClass

I took these steps:

Stacktrace:

#_app_views_flashes_edits__action_html_haml___358980396996600369_70093144333960 app/views/flashes/edits/_action.html.haml, line 5 block (2 levels) in ##_app_views_shared__flash_html_haml__2686093040528584098_70093147117380 app/views/shared/_flash.html.haml, line 18 block in ##_app_views_shared__flash_html_haml__2686093040528584098_70093147117380 app/views/shared/_flash.html.haml, line 9 ##_app_views_shared__flash_html_haml__2686093040528584098_70093147117380 app/views/shared/_flash.html.haml, line 4 ##_app_views_layouts_application_html_haml___583069045447158368_70093148478740 app/views/layouts/application.html.haml, line 12

This may be related to another issue, but I'm not sure which one. It seems to emanate from the flash message, not anything else...

allthesignals commented 7 years ago

@mzagaja pushed branch addresses and closes #173

allthesignals commented 7 years ago

@mzagaja reverted, sorry!

mzagaja commented 7 years ago

@allthesignals So this issue reproduces in develop as well. Here are the steps I can take to do so:

  1. Start with a fresh database per the README.md
  2. Make a new user
  3. Create a new organization that the user belongs to
  4. Create a new development from the organization page where it has the green button to "create development" (not sure if this is different from doing it from the main UI).
  5. Fill out dummy information for new development. Specifically using almost all 0s and some test description and tagline that validates. I used 60 Temple Pl, Boston for the test address.
  6. Note that URL is empty. Click the edit button and propose changing the URL to "http://www.cnn.com/"
  7. In Postico link the new user account to the development by adding the following entry to the development_team_members table: 143 1 33 13 2016-09-09 00:00:00 2016-09-09 00:00:00.
  8. Refresh the app in the web browser and see the notification to moderate the edit as proposed.
  9. Approve the edit. A message comes up complaining about a conflict about the location.
  10. Receive the following error: screen shot 2016-12-27 at 2 13 46 pm

I think this might be its own issue so investigating it further.

allthesignals commented 7 years ago

@mzagaja This looks good, I would just say that when another user proposes a change, and I approve that change, the flash isn't correct, but this is super minimal: screen shot 2016-12-27 at 3 08 32 pm

It's your call whether that should be quashed or not!

mzagaja commented 7 years ago

Oh I get what you're saying. I was using the same user to approve and make the edit so I didn't realize it was a flash message for the approver. So it should really be something like "You successfully approved the edit."?

allthesignals commented 7 years ago

@mzagaja Yes. Or "You approved {username}'s edit." But that might be a little more work!