Allows check-ins of unreassignable licenses #16063

Godmartinz · 2025-01-13T22:40:48Z

This adds the ability to check in unreassignable licenses.
When a seat is checked in, the seat is disabled and a default note of Seat has been used. will be added in addition to any other note entered.
On check in of a unreassignable license the seat is marked dead and also will not be allowed to be checked out via the API.

The available seat count is also updated accordingly on the seats tab, the index page, and the licenses reports.

API attempt to checkout an unreassignable:

[sc-14999]

what-the-diff · 2025-01-13T22:41:18Z

PR Summary

Introduction of New Model References
Imported Actionlog and LicenseSeat models into the Helper.php file to allow for advanced functionality.
Addition of New Method
Created a new method unReassignableCount($license) in Helper.php that tallies up the number of licenses that can't be reassigned.
Revised LicenseCheckinController:
- Altered the license check-in control, removing the need to check if a license is reassignable before checking-in.
- Enhanced the note-keeping for licenses that can't be reassigned, allowing for more accurate record-keeping during the check-in process.
- Changed how events are initiated to consider the revised notes.
Upgraded LicensesController:
- Refactored computation of checkedout_seats_count to consider licenses that can't be reassigned.
- Following this, unreassignable_seats_count is sent for viewing.
Transformation of LicenseSeatsTransformer:
- Incorporated a new method unReassignable($seat) to verify if a seat is non-reassignable.
- Altered the transformation output to feature a 'disabled' field.
Modification of LicensesTransformer:
- Tweaked the computation of free_seats_count to deduct the count of unReassignable licenses.
Revision of License Model:
- Altered remaincount() to subtract unreassignable licenses, providing a more accurate count of available licenses.
Upgraded Blade Templates:
- Optimized the licenses' view to display available license count subject to unreassignable seats.
- Updated the interaction with the check-in button when a seat is disabled as depicted in bootstrap-table.
Language Files Update:
- Enhanced clarity by updating the message for non-reassignable licenses to 'Seat has been used' instead.

Godmartinz · 2025-01-13T22:44:53Z

disabled button color has been fixed 👍

Godmartinz · 2025-01-13T22:47:19Z

looking into failing tests

app/Http/Controllers/Api/LicenseSeatsController.php

snipe · 2025-01-16T19:56:31Z

Not sure I love the field dead for this.

snipe · 2025-01-16T19:57:10Z

(Also, tests are failing)

marcusmoore · 2025-01-16T19:57:17Z

Not sure I love the field dead for this.

😆 @Godmartinz and I were going back and forth on how best to communicate what the column represents.

snipe · 2025-01-16T20:00:09Z

@marcusmoore - unassignable would probably work fine, but I'd probably return a true or false if it's meant to be boolean. Or reverse the logic and use reassignable.

marcusmoore · 2025-01-16T20:10:35Z

@snipe we (me really) were trying to avoid confusion around a license seat having assigned_to and unassignable (or a similar word) both be populated at the same time. That really threw me off and I wanted to find another word to signify "this seat might be checked out now but it cannot be checked out again in the future".

Maybe other devs won't find that as confusing as I did.

snipe · 2025-01-16T20:11:32Z

@marcusmoore reassignable_seat perhaps?

marcusmoore · 2025-01-16T20:13:04Z

@snipe that would be a lot clearer for me. Though I would go with reassignable so we don't have a redundant word in there $licenseSeat-> reassignable_seat // @Godmartinz

marcusmoore

The bones are solid but I think there is some improvements that can be made.

marcusmoore · 2025-01-16T21:33:59Z

app/Helpers/Helper.php

@@ -2,13 +2,15 @@

 namespace App\Helpers;
 use App\Models\Accessory;
+use App\Models\Actionlog;


This is unused and can be removed.

marcusmoore · 2025-01-16T21:36:11Z

app/Helpers/Helper.php

+    public static function unReassignableCount($license)
+    {
+
+        if (!$license->reassignable) {
+            $count = LicenseSeat::where('unreassignable_seat', '=', true)
+                ->where('license_id', '=', $license->id)
+                ->count();
+            return $count;
+        }
+    }


This method feels like it would be more at home on the actual License model. Is there any reason it needs to be a helper?

marcusmoore · 2025-01-16T21:39:34Z

database/migrations/2025_01_15_190348_adds_unavailable_to_license_seats_tables.php

@@ -0,0 +1,29 @@
+<?php
+namespace App\Models\LicenseSeat;
+use App\Models\LicenseSeat;


This unused import can be removed.

marcusmoore · 2025-01-16T21:39:41Z

database/migrations/2025_01_15_190348_adds_unavailable_to_license_seats_tables.php

+use App\Models\LicenseSeat;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;


This unused import can be removed.

marcusmoore · 2025-01-16T21:40:37Z

app/Models/License.php

I'd like to use proper casts more. Can we add $casts and set unreassignable_seat to be a boolean?

marcusmoore · 2025-01-16T21:44:49Z

app/Http/Controllers/Licenses/LicenseCheckinController.php

@@ -100,13 +95,18 @@ public function store(Request $request, $seatId = null, $backTo = null)
        $licenseSeat->assigned_to = null;
        $licenseSeat->asset_id = null;
        $licenseSeat->notes = $request->input('notes');
+        if (! $licenseSeat->license->reassignable) {
+            $licenseSeat->unreassignable_seat = true;
+            $licenseSeat->notes .= "\n" . trans('admin/licenses/message.checkin.not_reassignable') . '.';


I'm not sure we need to append this message. I think we can remove it? If we're going to keep it then it should be in the API controller too.

marcusmoore · 2025-01-16T21:46:40Z

app/Http/Controllers/Licenses/LicensesController.php

+        $unreassignable_seats_count = Helper::unReassignableCount($license);
+        if(!$license->reassignable){
+            $checkedout_seats_count = ($total_seats_count - $available_seats_count - $unreassignable_seats_count );
+        }
+        else {
+            $checkedout_seats_count = ($total_seats_count - $available_seats_count);
+        }


This logic feels like it should live on the model since there's a good chance we'll want to use it elsewhere in the future. We can avoid copy/pasting by moving it there.

marcusmoore · 2025-01-16T21:46:55Z

app/Http/Transformers/LicenseSeatsTransformer.php

+//    private function unReassignable($seat)
+//    {
+//        if (!$seat->license->reassignable) {
+//           $exists = Actionlog::where('action_type', '=', 'checkin from')
+//                ->where('item_id', '=', $seat->license->id)
+//                ->where('updated_at', '=', $seat->updated_at)
+//                ->exists();
+//           if($exists) {
+//               return true;
+//           }
+//            return false;
+//        }
+//    }


This can be removed.

marcusmoore · 2025-01-16T21:47:10Z

app/Http/Transformers/LicenseSeatsTransformer.php

@@ -2,6 +2,7 @@

 namespace App\Http\Transformers;

+use App\Models\Actionlog;


This unused import can be removed.

marcusmoore · 2025-01-16T21:47:57Z

app/Http/Transformers/LicensesTransformer.php

@@ -7,7 +7,7 @@
 use Illuminate\Support\Facades\Gate;
 use Illuminate\Database\Eloquent\Collection;

-class LicensesTransformer
+class  LicensesTransformer


This extra space can be removed.

Godmartinz added 3 commits January 13, 2025 13:35

allows checkin of unreassignable license seats

8a0ed49

adds count on tabs and reports and index

5e1562a

remove unrelated change

50e0e4a

Godmartinz requested a review from snipe as a code owner January 13, 2025 22:40

probot-autolabeler bot added backend frontend translations labels Jan 13, 2025

Godmartinz requested review from marcusmoore and snipe and removed request for snipe January 13, 2025 22:41

Godmartinz requested a review from spencerrlongg January 13, 2025 22:41

get button color correct

9da15a8

Godmartinz marked this pull request as draft January 14, 2025 20:24

Godmartinz added 2 commits January 15, 2025 15:36

adds migration for column unavailable, changes logic to utlize value

7a23372

fixes test to check if checked in licenses is unavailable

344b4e7

probot-autolabeler bot added the tests label Jan 16, 2025

updated column name, updated Api license checkin and out

f47a2b1

probot-autolabeler bot added the api label Jan 16, 2025

Godmartinz marked this pull request as ready for review January 16, 2025 19:53

snipe reviewed Jan 16, 2025

View reviewed changes

app/Http/Controllers/Api/LicenseSeatsController.php Outdated Show resolved Hide resolved

fixed typo, fixed variable name

db11fc3

renamed dead to unassigned, replaced 0 and 1 with true and flase

f050864

Godmartinz added 4 commits January 16, 2025 12:14

fixed factory and test

3f3f2bf

renamed unassignable to reassignable_seat

52bf0fa

last rename

d217c2e

remove unintended change

6bab6e7

Godmartinz requested a review from snipe January 16, 2025 20:29

marcusmoore requested changes Jan 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allows check-ins of unreassignable licenses #16063

Allows check-ins of unreassignable licenses #16063

Godmartinz commented Jan 13, 2025 •

edited

Loading

what-the-diff bot commented Jan 13, 2025

Godmartinz commented Jan 13, 2025

Godmartinz commented Jan 13, 2025

snipe commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025 •

edited

Loading

marcusmoore left a comment

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

marcusmoore Jan 16, 2025

		@@ -2,6 +2,7 @@

		namespace App\Http\Transformers;

		use App\Models\Actionlog;

Allows check-ins of unreassignable licenses #16063

Are you sure you want to change the base?

Allows check-ins of unreassignable licenses #16063

Conversation

Godmartinz commented Jan 13, 2025 • edited Loading

what-the-diff bot commented Jan 13, 2025

PR Summary

Godmartinz commented Jan 13, 2025

Godmartinz commented Jan 13, 2025

snipe commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025

snipe commented Jan 16, 2025

marcusmoore commented Jan 16, 2025 • edited Loading

marcusmoore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Godmartinz commented Jan 13, 2025 •

edited

Loading

marcusmoore commented Jan 16, 2025 •

edited

Loading