Approve all / select all #157
Conversation
|
Added "Deleted All" to checked entries. Also fixed a small bug on input array. |
| @@ -59,13 +59,13 @@ function submitEntry(obj, id) { | |||
| let gasoline = Number(!($(".mapboxgl-popup-content").find('.gasoline img').hasClass('no-gas'))); | |||
| let diesel = Number(!($(".mapboxgl-popup-content").find('.diesel img').hasClass('no-gas'))); | |||
| let lpg = Number(!($(".mapboxgl-popup-content").find('.lpg img').hasClass('no-gas'))); | |||
| validateCaptcha((token) => { | |||
| // validateCaptcha((token) => { | |||
There was a problem hiding this comment.
Commented code should be removed.
| @@ -75,7 +75,7 @@ function submitEntry(obj, id) { | |||
| $.post("/panel/entries/add", data, function (reply) { | |||
| console.log("Entrada adicionada: " + reply.success + " (0 -> falha, 1 -> sucesso)"); | |||
There was a problem hiding this comment.
console.log() calls should be removed. If you want to debug while developing, use the debugger construct, instead. And don't forget to remove it, once the code is ready for merging.
| let data = { | ||
| "fuel_station": id, | ||
| "gasoline": gasoline, | ||
| "diesel": diesel, | ||
| "lpg": lpg, | ||
| "captcha": token | ||
| // "captcha": token |
There was a problem hiding this comment.
Commented code should be removed.
| @@ -75,7 +75,7 @@ function submitEntry(obj, id) { | |||
| $.post("/panel/entries/add", data, function (reply) { | |||
| console.log("Entrada adicionada: " + reply.success + " (0 -> falha, 1 -> sucesso)"); | |||
| }, "json"); | |||
| }); | |||
| // }); | |||
| $cacheController = new CacheController(); | ||
| $cacheController->updateStations(); | ||
|
|
||
| return redirect('/panel/entries/list')->with('status', 'Entrada Validada Manualmente!'); |
There was a problem hiding this comment.
This route has a name set (entries.list). It's good practice to use those, instead of passing the URI.
You probably want to add a name to go along with the panel prefix in the routes, so the route name gets set to: panel.entries.list
| @@ -79,16 +79,50 @@ public function add(Request $request) | |||
|
|
|||
| public function push(Request $request) | |||
| { | |||
|
|
|||
There was a problem hiding this comment.
This blank line can be removed.
| $fuel_station = $entry->fuelStation(); | ||
| $fuel_station->update(['has_gasoline' => $entry->has_gasoline,'has_diesel' => $entry->has_diesel, 'has_lpg' => $entry->has_lpg]); | ||
| } catch (Exception $e) { | ||
| return redirect('/panel/entries/list')->with('status', 'Erro ao validar entrada!'); |
There was a problem hiding this comment.
Show the invalid id in the error message, when redirecting. Otherwise the user has no idea what entry failed.
| } | ||
|
|
||
| $cacheController = new CacheController(); | ||
| $cacheController->updateStations(); |
There was a problem hiding this comment.
Please, refactor this. Controllers shouldn't be used this way. The logic in the updateStations() method should be moved to a service class, which can then be called from the CacheController and other controllers, but not this way.
|
|
||
| foreach(request()->get('id') as $id){ | ||
| try { | ||
| $entry = Entry::findOrFail($id); |
There was a problem hiding this comment.
Please move this logic to an EntryService, EntryRepository or whatever class name you want to use, then inject said class into this controller method and run the update() method from here.
|
|
||
| foreach(request()->get('id') as $id){ | ||
| try { | ||
| \Log::info("deleting ".json_encode($id)); |
There was a problem hiding this comment.
The comments above, also apply in this method.
| @@ -59,13 +59,13 @@ function submitEntry(obj, id) { | |||
| let gasoline = Number(!($(".mapboxgl-popup-content").find('.gasoline img').hasClass('no-gas'))); | |||
| let diesel = Number(!($(".mapboxgl-popup-content").find('.diesel img').hasClass('no-gas'))); | |||
| let lpg = Number(!($(".mapboxgl-popup-content").find('.lpg img').hasClass('no-gas'))); | |||
| validateCaptcha((token) => { | |||
| // validateCaptcha((token) => { | |||
| let data = { | |||
There was a problem hiding this comment.
Are we leaving IE users (among others) out on purpose?
| let data = { | ||
| "fuel_station": id, | ||
| "gasoline": gasoline, | ||
| "diesel": diesel, | ||
| "lpg": lpg, | ||
| "captcha": token | ||
| // "captcha": token | ||
| } | ||
| $(obj).parent().parent().find('.popup_submit_text').html("VALIDADO"); |
There was a problem hiding this comment.
This approach is very dangerous. It is too bounded to markup. If a new layer is added/removed it will break.
Instead we should use a selector to navigate up on the tree to grab the desired element.
#96