Closed neodes-h closed 6 years ago
I wanted to make several changes on top of your stuff and thought it was faster to do them myself so I merged them with pull request #632.
Beyond what you did, I:
made a separate commit for adding the bootstrap.js file. A separate commit seemed like the best way to explain why the third party library was added to the project.
removed the merge commits you had. It looks like you're merging instead of rebasing when you get latest. See "Say you want to get latest changes on a branch you’re working on" at https://github.com/hhaccessibility/hhaccessibility.github.io/wiki/git-commands.
replaced usage of window.alert method with adding the validation messages and success messages into the modal element. If you hit "Confirm", you'll see what this means. Messages in the doucment are styled better and look better than anything JavaScript's standard "alert" method can produce.
moved Suggestion API method from LocationReportController to a new SuggestionController. LocationReportController had a lot of functionality already and there will be more suggestion features to come that will add nicely to a more focused SuggestionController.
added a couple more tests for getting the location report and checking that the suggestion feature can be found in it
Converted the API to respond with HTTP error status codes like 403 and 422 when validation problems happen. You had the API returning 200 (OK) responses even when the user was signed in or parameters were invalid. There's nothing big wrong with that except that it doesn't fit as nicely with what HTTP status codes are supposed to do.
Changed the success in the API response from 0, 1, 2, to strictly 0 or 1 where 1 happens exactly when an HTTP status code of 200 is returned. The success: 2 just seemed odd when the property name suggests that it means something was either successful or unsuccessful(1 or 0).
Moved some of the markup for the suggestion form to a new file called suggestion_form.blade.php. I did this in case we want a full page alternative to the modal you created. JavaScript like the stuff you did to show and hide the modal doesn't work so well with screen reading software so we may eventually want a full screen version of the suggestion form for users who indicated in their profiles that they use screen readers.
I'll close this because your changes are merged by #632 but it just also includes some things I did.