Users adding each other in Laravel

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite












I'm looking for a code review for the following system that I created. It allows a landlord, to add a tenant to start a tenancy. It has added, accept, reject, and prohibits a tenant from adding themselves. Tenants can't add landlords.



First thing up is my controller



The form is what is rendered when the landlord clicks add. It prepopulates with landlord name and id. The tenant's name, and id, and also the landlord's properties.



Create a Tenancy



These are the columns in the db



 //Renders Form
public function create($id)
$user = User::where('id', $id)->first();
$properties = PropertyAdvert::where('user_id', Auth::id())->get();

return view('/pages/account/tenancy/create', compact('user', 'properties'));


//Stores data
public function store(Request $request, User $user)
$properties = PropertyAdvert::where('user_id', Auth::id())->get();
$Tenancy = Tenancy::create([
'tenant_id' => $request->tenant_id,
'tenant_name' => $request->tenant_name,
'landlord_id' => $request->landlord_id,
'landlord_name' => $request->landlord_name,
'property_address' => $request->property_address,
]);

//Redirct somewhere


public function accept(Request $request)
Tenancy::where('accepted', 0)->where('request_sent', 1)
->update(
[
'accepted' => 1,
'request_sent' => 0,
]

);
return back();


public function reject(Request $request)
Tenancy::where('accepted', 0)->where('request_sent', 1)
->update(
[
'accepted' => 0,
'request_sent' => 0,
]
);
return back();


public function end(Request $request)
Tenancy::where('accepted', 1)->update(
[
'accepted' => 0,
]
);
return back();



This is the view
This is the tenants page. Only the landlord can add, but only the tenant can accept/reject and end the tenancy. Different buttons appear based on the bolleans in the db



 <div class="container">

<!-- ============== Tenant Profile section ============== -->

@if($user->userType != "Landlord")
<div class="row">
<!--
If the request hasn't been sent, and hasn't been excepted.
Provie button to send a tenancy request
-->

<!--
If tennancy = null
if tennacny not accepted
if request hasn't been sent
if currently signed in user != user/tenant id
-->
@if(Auth::user()->id == $user->id)
<h1>You cannot add yourself</h1>
@elseif($Tenancy == null || $Tenancy->accepted == 0 && $Tenancy->request_sent != 1)
<a href="/account/tenancy/$user->id/create" class="btn btn-primary">Start Tenancy</a>
@endif

<!--
If the user signed in, isn't the owner of this profile.
Do not show these buttons that control accept/reject/end
-->

@if(Auth::user()->id == $user->id)
<!--
If the request has been sent, but hasn't been accepted.
Give option to accept and reject.
This updates the values in DB.
-->
@if($Tenancy != null && $Tenancy->accepted == 0 && $Tenancy->request_sent == 1)
<form method="POST" action="/account/tenancy/$user->id/accept">
csrf_field()
<input type="submit" class="btn btn-primary" value="Accept Request">
</form>
<form method="POST" action="/account/tenancy/$user->id/reject">
csrf_field()
<input type="submit" class="btn btn-warning" value="Reject Request">
</form>
<!--
If the request has been accepted.
Show button to end the tenancy,
and property details
-->
@elseif($Tenancy != null && $Tenancy->accepted == 1 && $Tenancy->request_sent == 0)
<form method="POST" action="/account/tenancy/$user->id/end">
csrf_field()
<input type="submit" class="btn btn-primary" value="End Tenancy">
</form>
<h5>Currently in Tenancy with $Tenancy->landlord_name</h5>
<h5>Your property is $Tenancy->property_address</h5>
@endif <!-- End of current user vs this user-->
@endif <!-- Initial If-->
</div>

<div class="row mt-4">
<div class="col-md-9">
<span class="text-lead text-center">Your watched properties</span>
<hr>
<div class="row py-2">
@foreach ($user->WatchedProperties as $WatchedProperties)
<div class="col-md-4 mb-4">
<a href="/property/$WatchedProperties->image_info">
<img class="list-image img-fluid" src="$WatchedProperties->image_url">
</a>
<p class="mt-2">$WatchedProperties->address .', '. $WatchedProperties->town .', '. $WatchedProperties->county</p>
</div>
@endforeach
</div>
</div>
<div class="col-md-3 spacing">
<table class="table">
<tbody>
<!--
Looping thorugh all watchlists.
Watchlist controller Index
-->
<a href="#" class="link-sub-title">Property Preferences</a>
<p class="text-sub-title">Your Watchlists</p>
@foreach ($Watchlists as $Watchlist)
<tr>
<td>
<a href="/watchlist/$Watchlist->id">
$Watchlist->title
</a>
</td>
</tr>
@endforeach
</tbody>
</table>
</div>
</div>






share|improve this question



























    up vote
    4
    down vote

    favorite












    I'm looking for a code review for the following system that I created. It allows a landlord, to add a tenant to start a tenancy. It has added, accept, reject, and prohibits a tenant from adding themselves. Tenants can't add landlords.



    First thing up is my controller



    The form is what is rendered when the landlord clicks add. It prepopulates with landlord name and id. The tenant's name, and id, and also the landlord's properties.



    Create a Tenancy



    These are the columns in the db



     //Renders Form
    public function create($id)
    $user = User::where('id', $id)->first();
    $properties = PropertyAdvert::where('user_id', Auth::id())->get();

    return view('/pages/account/tenancy/create', compact('user', 'properties'));


    //Stores data
    public function store(Request $request, User $user)
    $properties = PropertyAdvert::where('user_id', Auth::id())->get();
    $Tenancy = Tenancy::create([
    'tenant_id' => $request->tenant_id,
    'tenant_name' => $request->tenant_name,
    'landlord_id' => $request->landlord_id,
    'landlord_name' => $request->landlord_name,
    'property_address' => $request->property_address,
    ]);

    //Redirct somewhere


    public function accept(Request $request)
    Tenancy::where('accepted', 0)->where('request_sent', 1)
    ->update(
    [
    'accepted' => 1,
    'request_sent' => 0,
    ]

    );
    return back();


    public function reject(Request $request)
    Tenancy::where('accepted', 0)->where('request_sent', 1)
    ->update(
    [
    'accepted' => 0,
    'request_sent' => 0,
    ]
    );
    return back();


    public function end(Request $request)
    Tenancy::where('accepted', 1)->update(
    [
    'accepted' => 0,
    ]
    );
    return back();



    This is the view
    This is the tenants page. Only the landlord can add, but only the tenant can accept/reject and end the tenancy. Different buttons appear based on the bolleans in the db



     <div class="container">

    <!-- ============== Tenant Profile section ============== -->

    @if($user->userType != "Landlord")
    <div class="row">
    <!--
    If the request hasn't been sent, and hasn't been excepted.
    Provie button to send a tenancy request
    -->

    <!--
    If tennancy = null
    if tennacny not accepted
    if request hasn't been sent
    if currently signed in user != user/tenant id
    -->
    @if(Auth::user()->id == $user->id)
    <h1>You cannot add yourself</h1>
    @elseif($Tenancy == null || $Tenancy->accepted == 0 && $Tenancy->request_sent != 1)
    <a href="/account/tenancy/$user->id/create" class="btn btn-primary">Start Tenancy</a>
    @endif

    <!--
    If the user signed in, isn't the owner of this profile.
    Do not show these buttons that control accept/reject/end
    -->

    @if(Auth::user()->id == $user->id)
    <!--
    If the request has been sent, but hasn't been accepted.
    Give option to accept and reject.
    This updates the values in DB.
    -->
    @if($Tenancy != null && $Tenancy->accepted == 0 && $Tenancy->request_sent == 1)
    <form method="POST" action="/account/tenancy/$user->id/accept">
    csrf_field()
    <input type="submit" class="btn btn-primary" value="Accept Request">
    </form>
    <form method="POST" action="/account/tenancy/$user->id/reject">
    csrf_field()
    <input type="submit" class="btn btn-warning" value="Reject Request">
    </form>
    <!--
    If the request has been accepted.
    Show button to end the tenancy,
    and property details
    -->
    @elseif($Tenancy != null && $Tenancy->accepted == 1 && $Tenancy->request_sent == 0)
    <form method="POST" action="/account/tenancy/$user->id/end">
    csrf_field()
    <input type="submit" class="btn btn-primary" value="End Tenancy">
    </form>
    <h5>Currently in Tenancy with $Tenancy->landlord_name</h5>
    <h5>Your property is $Tenancy->property_address</h5>
    @endif <!-- End of current user vs this user-->
    @endif <!-- Initial If-->
    </div>

    <div class="row mt-4">
    <div class="col-md-9">
    <span class="text-lead text-center">Your watched properties</span>
    <hr>
    <div class="row py-2">
    @foreach ($user->WatchedProperties as $WatchedProperties)
    <div class="col-md-4 mb-4">
    <a href="/property/$WatchedProperties->image_info">
    <img class="list-image img-fluid" src="$WatchedProperties->image_url">
    </a>
    <p class="mt-2">$WatchedProperties->address .', '. $WatchedProperties->town .', '. $WatchedProperties->county</p>
    </div>
    @endforeach
    </div>
    </div>
    <div class="col-md-3 spacing">
    <table class="table">
    <tbody>
    <!--
    Looping thorugh all watchlists.
    Watchlist controller Index
    -->
    <a href="#" class="link-sub-title">Property Preferences</a>
    <p class="text-sub-title">Your Watchlists</p>
    @foreach ($Watchlists as $Watchlist)
    <tr>
    <td>
    <a href="/watchlist/$Watchlist->id">
    $Watchlist->title
    </a>
    </td>
    </tr>
    @endforeach
    </tbody>
    </table>
    </div>
    </div>






    share|improve this question























      up vote
      4
      down vote

      favorite









      up vote
      4
      down vote

      favorite











      I'm looking for a code review for the following system that I created. It allows a landlord, to add a tenant to start a tenancy. It has added, accept, reject, and prohibits a tenant from adding themselves. Tenants can't add landlords.



      First thing up is my controller



      The form is what is rendered when the landlord clicks add. It prepopulates with landlord name and id. The tenant's name, and id, and also the landlord's properties.



      Create a Tenancy



      These are the columns in the db



       //Renders Form
      public function create($id)
      $user = User::where('id', $id)->first();
      $properties = PropertyAdvert::where('user_id', Auth::id())->get();

      return view('/pages/account/tenancy/create', compact('user', 'properties'));


      //Stores data
      public function store(Request $request, User $user)
      $properties = PropertyAdvert::where('user_id', Auth::id())->get();
      $Tenancy = Tenancy::create([
      'tenant_id' => $request->tenant_id,
      'tenant_name' => $request->tenant_name,
      'landlord_id' => $request->landlord_id,
      'landlord_name' => $request->landlord_name,
      'property_address' => $request->property_address,
      ]);

      //Redirct somewhere


      public function accept(Request $request)
      Tenancy::where('accepted', 0)->where('request_sent', 1)
      ->update(
      [
      'accepted' => 1,
      'request_sent' => 0,
      ]

      );
      return back();


      public function reject(Request $request)
      Tenancy::where('accepted', 0)->where('request_sent', 1)
      ->update(
      [
      'accepted' => 0,
      'request_sent' => 0,
      ]
      );
      return back();


      public function end(Request $request)
      Tenancy::where('accepted', 1)->update(
      [
      'accepted' => 0,
      ]
      );
      return back();



      This is the view
      This is the tenants page. Only the landlord can add, but only the tenant can accept/reject and end the tenancy. Different buttons appear based on the bolleans in the db



       <div class="container">

      <!-- ============== Tenant Profile section ============== -->

      @if($user->userType != "Landlord")
      <div class="row">
      <!--
      If the request hasn't been sent, and hasn't been excepted.
      Provie button to send a tenancy request
      -->

      <!--
      If tennancy = null
      if tennacny not accepted
      if request hasn't been sent
      if currently signed in user != user/tenant id
      -->
      @if(Auth::user()->id == $user->id)
      <h1>You cannot add yourself</h1>
      @elseif($Tenancy == null || $Tenancy->accepted == 0 && $Tenancy->request_sent != 1)
      <a href="/account/tenancy/$user->id/create" class="btn btn-primary">Start Tenancy</a>
      @endif

      <!--
      If the user signed in, isn't the owner of this profile.
      Do not show these buttons that control accept/reject/end
      -->

      @if(Auth::user()->id == $user->id)
      <!--
      If the request has been sent, but hasn't been accepted.
      Give option to accept and reject.
      This updates the values in DB.
      -->
      @if($Tenancy != null && $Tenancy->accepted == 0 && $Tenancy->request_sent == 1)
      <form method="POST" action="/account/tenancy/$user->id/accept">
      csrf_field()
      <input type="submit" class="btn btn-primary" value="Accept Request">
      </form>
      <form method="POST" action="/account/tenancy/$user->id/reject">
      csrf_field()
      <input type="submit" class="btn btn-warning" value="Reject Request">
      </form>
      <!--
      If the request has been accepted.
      Show button to end the tenancy,
      and property details
      -->
      @elseif($Tenancy != null && $Tenancy->accepted == 1 && $Tenancy->request_sent == 0)
      <form method="POST" action="/account/tenancy/$user->id/end">
      csrf_field()
      <input type="submit" class="btn btn-primary" value="End Tenancy">
      </form>
      <h5>Currently in Tenancy with $Tenancy->landlord_name</h5>
      <h5>Your property is $Tenancy->property_address</h5>
      @endif <!-- End of current user vs this user-->
      @endif <!-- Initial If-->
      </div>

      <div class="row mt-4">
      <div class="col-md-9">
      <span class="text-lead text-center">Your watched properties</span>
      <hr>
      <div class="row py-2">
      @foreach ($user->WatchedProperties as $WatchedProperties)
      <div class="col-md-4 mb-4">
      <a href="/property/$WatchedProperties->image_info">
      <img class="list-image img-fluid" src="$WatchedProperties->image_url">
      </a>
      <p class="mt-2">$WatchedProperties->address .', '. $WatchedProperties->town .', '. $WatchedProperties->county</p>
      </div>
      @endforeach
      </div>
      </div>
      <div class="col-md-3 spacing">
      <table class="table">
      <tbody>
      <!--
      Looping thorugh all watchlists.
      Watchlist controller Index
      -->
      <a href="#" class="link-sub-title">Property Preferences</a>
      <p class="text-sub-title">Your Watchlists</p>
      @foreach ($Watchlists as $Watchlist)
      <tr>
      <td>
      <a href="/watchlist/$Watchlist->id">
      $Watchlist->title
      </a>
      </td>
      </tr>
      @endforeach
      </tbody>
      </table>
      </div>
      </div>






      share|improve this question













      I'm looking for a code review for the following system that I created. It allows a landlord, to add a tenant to start a tenancy. It has added, accept, reject, and prohibits a tenant from adding themselves. Tenants can't add landlords.



      First thing up is my controller



      The form is what is rendered when the landlord clicks add. It prepopulates with landlord name and id. The tenant's name, and id, and also the landlord's properties.



      Create a Tenancy



      These are the columns in the db



       //Renders Form
      public function create($id)
      $user = User::where('id', $id)->first();
      $properties = PropertyAdvert::where('user_id', Auth::id())->get();

      return view('/pages/account/tenancy/create', compact('user', 'properties'));


      //Stores data
      public function store(Request $request, User $user)
      $properties = PropertyAdvert::where('user_id', Auth::id())->get();
      $Tenancy = Tenancy::create([
      'tenant_id' => $request->tenant_id,
      'tenant_name' => $request->tenant_name,
      'landlord_id' => $request->landlord_id,
      'landlord_name' => $request->landlord_name,
      'property_address' => $request->property_address,
      ]);

      //Redirct somewhere


      public function accept(Request $request)
      Tenancy::where('accepted', 0)->where('request_sent', 1)
      ->update(
      [
      'accepted' => 1,
      'request_sent' => 0,
      ]

      );
      return back();


      public function reject(Request $request)
      Tenancy::where('accepted', 0)->where('request_sent', 1)
      ->update(
      [
      'accepted' => 0,
      'request_sent' => 0,
      ]
      );
      return back();


      public function end(Request $request)
      Tenancy::where('accepted', 1)->update(
      [
      'accepted' => 0,
      ]
      );
      return back();



      This is the view
      This is the tenants page. Only the landlord can add, but only the tenant can accept/reject and end the tenancy. Different buttons appear based on the bolleans in the db



       <div class="container">

      <!-- ============== Tenant Profile section ============== -->

      @if($user->userType != "Landlord")
      <div class="row">
      <!--
      If the request hasn't been sent, and hasn't been excepted.
      Provie button to send a tenancy request
      -->

      <!--
      If tennancy = null
      if tennacny not accepted
      if request hasn't been sent
      if currently signed in user != user/tenant id
      -->
      @if(Auth::user()->id == $user->id)
      <h1>You cannot add yourself</h1>
      @elseif($Tenancy == null || $Tenancy->accepted == 0 && $Tenancy->request_sent != 1)
      <a href="/account/tenancy/$user->id/create" class="btn btn-primary">Start Tenancy</a>
      @endif

      <!--
      If the user signed in, isn't the owner of this profile.
      Do not show these buttons that control accept/reject/end
      -->

      @if(Auth::user()->id == $user->id)
      <!--
      If the request has been sent, but hasn't been accepted.
      Give option to accept and reject.
      This updates the values in DB.
      -->
      @if($Tenancy != null && $Tenancy->accepted == 0 && $Tenancy->request_sent == 1)
      <form method="POST" action="/account/tenancy/$user->id/accept">
      csrf_field()
      <input type="submit" class="btn btn-primary" value="Accept Request">
      </form>
      <form method="POST" action="/account/tenancy/$user->id/reject">
      csrf_field()
      <input type="submit" class="btn btn-warning" value="Reject Request">
      </form>
      <!--
      If the request has been accepted.
      Show button to end the tenancy,
      and property details
      -->
      @elseif($Tenancy != null && $Tenancy->accepted == 1 && $Tenancy->request_sent == 0)
      <form method="POST" action="/account/tenancy/$user->id/end">
      csrf_field()
      <input type="submit" class="btn btn-primary" value="End Tenancy">
      </form>
      <h5>Currently in Tenancy with $Tenancy->landlord_name</h5>
      <h5>Your property is $Tenancy->property_address</h5>
      @endif <!-- End of current user vs this user-->
      @endif <!-- Initial If-->
      </div>

      <div class="row mt-4">
      <div class="col-md-9">
      <span class="text-lead text-center">Your watched properties</span>
      <hr>
      <div class="row py-2">
      @foreach ($user->WatchedProperties as $WatchedProperties)
      <div class="col-md-4 mb-4">
      <a href="/property/$WatchedProperties->image_info">
      <img class="list-image img-fluid" src="$WatchedProperties->image_url">
      </a>
      <p class="mt-2">$WatchedProperties->address .', '. $WatchedProperties->town .', '. $WatchedProperties->county</p>
      </div>
      @endforeach
      </div>
      </div>
      <div class="col-md-3 spacing">
      <table class="table">
      <tbody>
      <!--
      Looping thorugh all watchlists.
      Watchlist controller Index
      -->
      <a href="#" class="link-sub-title">Property Preferences</a>
      <p class="text-sub-title">Your Watchlists</p>
      @foreach ($Watchlists as $Watchlist)
      <tr>
      <td>
      <a href="/watchlist/$Watchlist->id">
      $Watchlist->title
      </a>
      </td>
      </tr>
      @endforeach
      </tbody>
      </table>
      </div>
      </div>








      share|improve this question












      share|improve this question




      share|improve this question








      edited Apr 28 at 21:40









      200_success

      123k14142399




      123k14142399









      asked Apr 28 at 14:34









      jynk

      211




      211




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          1
          down vote













          Magic strings



          You are using the magic string "landlord". Consider using a static variable, for example on your user model instead. Using a static variable has some advantages over using this magic string:



          • IDE's will allow you to inspect where your variable is used

          • You have all the possible options in one place, preventing you from having to guess it

          • Changing the string is now an easy task, instead of a very tedious one where you have to search for it through your entire codebase

          Validate your requests



          I don't think you are validating anything right now. You prevent the forms from showing for certain users, but do not prevent the endpoints from accepting requests.



          Add casts in your models



          You can use $casts in your models to use booleans instead of ints for your accepted and request_sent variables on your Tenancy.



          Use blade comments



          You are currently using html comments, which will be rendered in the source of your page. Instead use blade comments (ala -- My comment --) which will be translated to php comments and thus be hidden.



          Some side notes



          Consider using @csrf instead of csrf_field() . The change is purely cosmetic though.



          I am assuming that you are using the VerifyCsrfToken and the auth middleware to prevent to verify the csrf token you added and make sure a user is actually logged in.






          share|improve this answer





















            Your Answer




            StackExchange.ifUsing("editor", function ()
            return StackExchange.using("mathjaxEditing", function ()
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            );
            );
            , "mathjax-editing");

            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: false,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );








             

            draft saved


            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f193150%2fusers-adding-each-other-in-laravel%23new-answer', 'question_page');

            );

            Post as a guest






























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            1
            down vote













            Magic strings



            You are using the magic string "landlord". Consider using a static variable, for example on your user model instead. Using a static variable has some advantages over using this magic string:



            • IDE's will allow you to inspect where your variable is used

            • You have all the possible options in one place, preventing you from having to guess it

            • Changing the string is now an easy task, instead of a very tedious one where you have to search for it through your entire codebase

            Validate your requests



            I don't think you are validating anything right now. You prevent the forms from showing for certain users, but do not prevent the endpoints from accepting requests.



            Add casts in your models



            You can use $casts in your models to use booleans instead of ints for your accepted and request_sent variables on your Tenancy.



            Use blade comments



            You are currently using html comments, which will be rendered in the source of your page. Instead use blade comments (ala -- My comment --) which will be translated to php comments and thus be hidden.



            Some side notes



            Consider using @csrf instead of csrf_field() . The change is purely cosmetic though.



            I am assuming that you are using the VerifyCsrfToken and the auth middleware to prevent to verify the csrf token you added and make sure a user is actually logged in.






            share|improve this answer

























              up vote
              1
              down vote













              Magic strings



              You are using the magic string "landlord". Consider using a static variable, for example on your user model instead. Using a static variable has some advantages over using this magic string:



              • IDE's will allow you to inspect where your variable is used

              • You have all the possible options in one place, preventing you from having to guess it

              • Changing the string is now an easy task, instead of a very tedious one where you have to search for it through your entire codebase

              Validate your requests



              I don't think you are validating anything right now. You prevent the forms from showing for certain users, but do not prevent the endpoints from accepting requests.



              Add casts in your models



              You can use $casts in your models to use booleans instead of ints for your accepted and request_sent variables on your Tenancy.



              Use blade comments



              You are currently using html comments, which will be rendered in the source of your page. Instead use blade comments (ala -- My comment --) which will be translated to php comments and thus be hidden.



              Some side notes



              Consider using @csrf instead of csrf_field() . The change is purely cosmetic though.



              I am assuming that you are using the VerifyCsrfToken and the auth middleware to prevent to verify the csrf token you added and make sure a user is actually logged in.






              share|improve this answer























                up vote
                1
                down vote










                up vote
                1
                down vote









                Magic strings



                You are using the magic string "landlord". Consider using a static variable, for example on your user model instead. Using a static variable has some advantages over using this magic string:



                • IDE's will allow you to inspect where your variable is used

                • You have all the possible options in one place, preventing you from having to guess it

                • Changing the string is now an easy task, instead of a very tedious one where you have to search for it through your entire codebase

                Validate your requests



                I don't think you are validating anything right now. You prevent the forms from showing for certain users, but do not prevent the endpoints from accepting requests.



                Add casts in your models



                You can use $casts in your models to use booleans instead of ints for your accepted and request_sent variables on your Tenancy.



                Use blade comments



                You are currently using html comments, which will be rendered in the source of your page. Instead use blade comments (ala -- My comment --) which will be translated to php comments and thus be hidden.



                Some side notes



                Consider using @csrf instead of csrf_field() . The change is purely cosmetic though.



                I am assuming that you are using the VerifyCsrfToken and the auth middleware to prevent to verify the csrf token you added and make sure a user is actually logged in.






                share|improve this answer













                Magic strings



                You are using the magic string "landlord". Consider using a static variable, for example on your user model instead. Using a static variable has some advantages over using this magic string:



                • IDE's will allow you to inspect where your variable is used

                • You have all the possible options in one place, preventing you from having to guess it

                • Changing the string is now an easy task, instead of a very tedious one where you have to search for it through your entire codebase

                Validate your requests



                I don't think you are validating anything right now. You prevent the forms from showing for certain users, but do not prevent the endpoints from accepting requests.



                Add casts in your models



                You can use $casts in your models to use booleans instead of ints for your accepted and request_sent variables on your Tenancy.



                Use blade comments



                You are currently using html comments, which will be rendered in the source of your page. Instead use blade comments (ala -- My comment --) which will be translated to php comments and thus be hidden.



                Some side notes



                Consider using @csrf instead of csrf_field() . The change is purely cosmetic though.



                I am assuming that you are using the VerifyCsrfToken and the auth middleware to prevent to verify the csrf token you added and make sure a user is actually logged in.







                share|improve this answer













                share|improve this answer



                share|improve this answer











                answered Apr 28 at 23:08









                Sumurai8

                2,260315




                2,260315






















                     

                    draft saved


                    draft discarded


























                     


                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f193150%2fusers-adding-each-other-in-laravel%23new-answer', 'question_page');

                    );

                    Post as a guest













































































                    Popular posts from this blog

                    Chat program with C++ and SFML

                    Function to Return a JSON Like Objects Using VBA Collections and Arrays

                    Will my employers contract hold up in court?