Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flights filtered per va #242

Merged
merged 5 commits into from
May 19, 2018
Merged

flights filtered per va #242

merged 5 commits into from
May 19, 2018

Conversation

LordWilbur
Copy link
Contributor

Pilots now see only their company's flights.

@nabeelio
Copy link
Owner

Sorry to be nitpicky, but no tabs, please. Indent with 4 spaces.

Also, create a new setting in the settings migration file, after 'pilots.only_flights_from_current:

        $this->addSetting('pilots.restrict_to_company', [
            'name'        => 'Restrict the flights to company',
            'group'       => 'pilots',
            'value'       => false,
            'type'        => 'boolean',
            'description' => 'Restrict flights to the user\'s airline',
        ]);

And check it before adding it to the where:

if(setting('pilots.restrict_to_company')) {
    $where['airline_id'] = Auth::user()->airline_id;
}

Leave the translations out for now, for the settings.

@@ -68,6 +69,8 @@ public function index(Request $request)

$flights = $this->flightRepo
->orderBy('flight_number', 'asc')
->orderBy('route_leg', 'asc')
->orderBy('route_code', 'asc')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave code out, for now

@@ -94,7 +97,7 @@ public function bids(Request $request)
$saved_flights = $flights->pluck('id')->toArray();

return view('flights.index', [
'title' => 'Bids',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave translations out until I fix the other commit. I'm rearranging some of the files.

@@ -110,8 +113,14 @@ public function bids(Request $request)
*/
public function search(Request $request)
{
$flights = $this->flightRepo->searchCriteria($request)->paginate();

$request['airline_id'] = Auth::user()->airline_id;
Copy link
Owner

@nabeelio nabeelio May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave this out from the $request. Instead, add as a separate $where, after checking if the setting is enabled. No tabs

@@ -52,7 +52,8 @@ public function __construct(
public function index(Request $request)
{
$where = [
'active' => true
'active' => true,
'airline_id' => Auth::user()->airline_id,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tabs

->orderBy('flight_number', 'asc')
->orderBy('route_leg', 'asc')
->orderBy('route_code', 'asc')
->paginate();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And no tabs

@nabeelio
Copy link
Owner

A test should also be added for this, in the ApiTest.php file, one that's with the setting enabled and one with it enabled. But I can do that later.

…ent = true and departure airport is not the same pilot's current airport.
@nabeelio
Copy link
Owner

nabeelio commented May 18, 2018

And just check with me first before adding features or whatever, I have a very specific way for implementing of a lot of them, or I have them completed already. This is an example of one I've already got done, but not pushed up yet... because there are changes to the API layer that I haven't written the tests for yet.

added setting
corrected code
@nabeelio
Copy link
Owner

Looks good, also needs to be done in app/Http/Controllers/Api/FlightController.php

@LordWilbur
Copy link
Contributor Author

All done my lord ;) Now i must go to work, leave me an email at pva0001@piemontevirtualairlines.tk if you want me to do something special, or i'll wait for your commit on translation to go forward with the admin panel translation.

@LordWilbur
Copy link
Contributor Author

Forgot to say: the menu icon on mobile devices still doesn't work and i have no clue on what to do to fix it...

@nabeelio nabeelio merged commit 77a2fd7 into nabeelio:dev May 19, 2018
@nabeelio
Copy link
Owner

Yeah I have to look at that. Merged, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants