Skip to content

Commit

Permalink
Fixes #222
Browse files Browse the repository at this point in the history
  • Loading branch information
snipe committed Aug 16, 2014
1 parent 13f61e1 commit 49c5607
Showing 1 changed file with 16 additions and 17 deletions.
33 changes: 16 additions & 17 deletions app/controllers/admin/LicensesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class LicensesController extends AdminController
*
* @return View
*/




public function getIndex()
Expand Down Expand Up @@ -336,8 +336,9 @@ public function getCheckout($seatId)
->whereNull('assets.deleted_at')
->get();

$asset_array = json_decode(json_encode($asset), true);

$asset_array = json_decode(json_encode($asset), true);
$asset_element[''] = 'Please select an asset';

// Build a list out of the data results
for ($x=0; $x<count($asset_array); $x++) {

Expand All @@ -346,12 +347,9 @@ public function getCheckout($seatId)
} else {
$full_name = ' (Unassigned)';
}
$asset_element[$asset_array[$x]['id']] = $asset_array[$x]['asset_tag'].' - '.$asset_array[$x]['name'].$full_name;


$asset_element[$asset_array[$x]['id']] = $asset_array[$x]['asset_tag'].' - '.$asset_array[$x]['name'].$full_name.' - '.$asset_array[$x]['id'];

}
//add label
array_splice($asset_element, 0, 0, "Please select an Asset");

This comment has been minimized.

Copy link
@snipe

snipe Aug 16, 2014

Author Owner

@technogenus this array_splice is what caused issue #222.


return View::make('backend/licenses/checkout', compact('licenseseat'))->with('users_list',$users_list)->with('asset_list',$asset_element);

Expand All @@ -364,7 +362,7 @@ public function getCheckout($seatId)
**/
public function postCheckout($seatId)
{


$assigned_to = e(Input::get('assigned_to'));
$asset_id = e(Input::get('asset_id'));
Expand All @@ -373,7 +371,8 @@ public function postCheckout($seatId)
$rules = array(

'note' => 'alpha_space',
'asset_id' => 'required_without:assigned_to:min:1',
// 'asset_id' => 'required_without:assigned_to:min:1',

This comment has been minimized.

Copy link
@snipe

snipe Aug 16, 2014

Author Owner

@technogenus this is commented out for now, since you guys removed the ability to assign to a user for now. When assign to use gets re-added, you will want to replace the new line below with the commented out line, so that one or the other must be selected.

'asset_id' => 'required',
);

// Create a new validator instance from our validation rules
Expand Down Expand Up @@ -541,26 +540,26 @@ public function getView($licenseId = null)
return Redirect::route('licenses')->with('error', $error);
}
}

public function getClone($licenseId = null)
{
// Check if the license exists
if (is_null($license_to_clone = License::find($licenseId))) {
// Redirect to the blogs management page
return Redirect::to('admin/licenses')->with('error', Lang::get('admin/licenses/message.does_not_exist'));
}

// Show the page
$license_options = array('0' => 'Top Level') + License::lists('name', 'id');

//clone the orig
$license = clone $license_to_clone;
$license->id = null;
$license->serial = null;
$license->id = null;
$license->serial = null;

// Show the page
$depreciation_list = array('0' => Lang::get('admin/licenses/form.no_depreciation')) + Depreciation::lists('name', 'id');
return View::make('backend/licenses/edit')->with('license_options',$license_options)->with('depreciation_list',$depreciation_list)->with('license',$license);

}
}

4 comments on commit 49c5607

@technogenus
Copy link

Choose a reason for hiding this comment

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

We could not find any reference to this change in our change log and also we did only basic tests of the delete asset function... probably missed this error all together as we didnt properly confirm which asset was deleted... most of the assets in our dev system have all similar test names and tags. Today we did confirm array_splice index is not being used anywhere else in the app to imitate actual object id.

@snipe
Copy link
Owner Author

@snipe snipe commented on 49c5607 Aug 16, 2014

Choose a reason for hiding this comment

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

array_splice resets the keys, that's why it was donking things up. I put a comment in on the line in your commit where it was added. No biggie, but we need to be careful about using array_splice in arrays where the key is actually the ID of an object

@technogenus
Copy link

Choose a reason for hiding this comment

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

Understood. I would be really surprised if any of our team used an array index as a database element ID reference (and would kinda like to know who...) but will definitely remind them.

@snipe
Copy link
Owner Author

@snipe snipe commented on 49c5607 Aug 16, 2014

Choose a reason for hiding this comment

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

I'm sure it was just a simple oversight. PHP's array handling can get funny sometimes. I think they were just trying to add the "Please select a blah" to the top of the array list (to build the array for the select menu) and forgot that that particular array function resets the keys. Simple mistake to make.

Please sign in to comment.