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.

Sign up for GitHub

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

Timeprofile copy fix #3835

Merged
merged 3 commits into from
May 10, 2018
Merged

Timeprofile copy fix #3835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1323d4
Timeprofile copy fixes
ZitaNemeckova Apr 24, 2018
cb132d5
Specs fixes
ZitaNemeckova Apr 24, 2018
39913cf
Pass specific action to js controller
ZitaNemeckova Apr 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions app/assets/javascripts/controllers/configuration/time_profile_form_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ManageIQ.angular.app.controller('timeProfileFormController', ['$http', 'timeProfileFormId', 'miqService', function($http, timeProfileFormId, miqService) {
ManageIQ.angular.app.controller('timeProfileFormController', ['$http', 'timeProfileFormId', 'timeProfileFormAction', 'miqService', function($http, timeProfileFormId, timeProfileFormAction, miqService) {
var vm = this;

var init = function() {
Expand All @@ -24,7 +24,6 @@ ManageIQ.angular.app.controller('timeProfileFormController', ['$http', 'timeProf
vm.dayNames = [__("Sunday"), __("Monday"), __("Tuesday"), __("Wednesday"), __("Thursday"), __("Friday"), __("Saturday")];
vm.hourNamesFirstHalf = [__("12-1"), __("1-2"), __("2-3"), __("3-4"), __("4-5"), __("5-6")];
vm.hourNamesSecondHalf = [__("6-7"), __("7-8"), __("8-9"), __("9-10"), __("10-11"), __("11-12")];
vm.formId = timeProfileFormId;
vm.afterGet = false;
vm.modelCopy = angular.copy( vm.timeProfileModel );
vm.model = 'timeProfileModel';
Expand All @@ -38,10 +37,12 @@ ManageIQ.angular.app.controller('timeProfileFormController', ['$http', 'timeProf
.then(getTimeProfileFormData)
.catch(miqService.handleFailure);

if (timeProfileFormId == 'new') {
vm.newRecord = true;
} else {
if (timeProfileFormAction === 'timeprofile_edit') {
vm.newRecord = false;
vm.formId = timeProfileFormId;
} else {
vm.newRecord = true;
vm.formId = "new";
}
};

Expand Down Expand Up @@ -167,7 +168,7 @@ ManageIQ.angular.app.controller('timeProfileFormController', ['$http', 'timeProf

var timeProfileEditButtonClicked = function(buttonName, serializeFields) {
miqService.sparkleOn();
var url = '/configuration/timeprofile_update/' + timeProfileFormId + '?button=' + buttonName;
var url = '/configuration/timeprofile_update/' + vm.formId + '?button=' + buttonName;
var timeProfileModelObj = angular.copy(vm.timeProfileModel);
miqService.miqAjaxButton(url, timeProfileModelObj);
};
Expand Down
15 changes: 4 additions & 11 deletions app/controllers/configuration_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ def get_hr_str(hr)
def timeprofile_new
assert_privileges("timeprofile_new")
@timeprofile = TimeProfile.new
@timeprofile_action = "timeprofile_new"
set_form_vars
@in_a_form = true
@breadcrumbs = []
Expand All @@ -253,6 +254,7 @@ def timeprofile_new
def timeprofile_edit
assert_privileges("tp_edit")
@timeprofile = TimeProfile.find(params[:id])
@timeprofile_action = "timeprofile_edit"
Copy link

@hstastna hstastna May 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@ZitaNemeckova The code looks good and it works. I just haven't checked the specs. Anyway, I wonder why we need https://github.com/ManageIQ/manageiq-ui-classic/pull/3835/files#diff-679158917bc83d666b7fa919b44817ebR246
and https://github.com/ManageIQ/manageiq-ui-classic/pull/3835/files#diff-679158917bc83d666b7fa919b44817ebR257 if this is already set in params[;action] - maybe we could use it instead of setting @timeprofile_action.. but maybe not :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's preventing Cross-Site Scripting (Hakiri is catching this kind of vulnerabilities). I cannot pass anything from params directly to haml.

Copy link

Choose a reason for hiding this comment

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

Ah, of course! Forgot.. Thanks! 👍

set_form_vars
@tp_restricted = true if @timeprofile.profile_type == "global" && !admin_user?
title = (@timeprofile.profile_type == "global" && !admin_user?) ? _("Time Profile") : _("Edit")
Expand Down Expand Up @@ -321,17 +323,8 @@ def timeprofile_copy
assert_privileges("tp_copy")
session[:set_copy] = "copy"
@in_a_form = true
timeprofile = TimeProfile.find(params[:id])
@timeprofile = TimeProfile.new
@timeprofile.description = _("Copy of %{description}") % {:description => timeprofile.description}
@timeprofile.profile_type = "user"
@timeprofile.profile_key = timeprofile.profile_key
unless timeprofile.profile.nil?
@timeprofile.profile ||= {}
@timeprofile.profile[:days] = timeprofile.profile[:days] if timeprofile.profile[:days]
@timeprofile.profile[:hours] = timeprofile.profile[:hours] if timeprofile.profile[:hours]
@timeprofile.profile[:tz] = timeprofile.profile[:tz] if timeprofile.profile[:tz]
end
@timeprofile = TimeProfile.find(params[:id])
@timeprofile_action = "timeprofile_copy"
set_form_vars
session[:changed] = false
drop_breadcrumb(:name => _("Adding copy of '%{description}'") % {:description => @timeprofile.description},
Expand Down
1 change: 1 addition & 0 deletions app/views/configuration/_timeprofile_form.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,5 @@

:javascript
ManageIQ.angular.app.value('timeProfileFormId', '#{@timeprofile.id || "new"}');
ManageIQ.angular.app.value('timeProfileFormAction', '#{@timeprofile_action}');
miq_bootstrap('#form_div');
73 changes: 65 additions & 8 deletions spec/javascripts/controllers/configuration/time_profile_form_controller_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('timeProfileFormController', function() {
$controller = _$controller_('timeProfileFormController as vm', {
$scope: $scope,
timeProfileFormId: 'new',
timeProfileFormAction: 'timeprofile_create',
miqService: miqService
});
}));
Expand All @@ -57,7 +58,7 @@ describe('timeProfileFormController', function() {
$httpBackend.flush();
});
describe('when the timeProfileFormId is new', function() {
it('sets the name to blank', function () {
it('sets the description to blank', function () {
expect($scope.vm.timeProfileModel.description).toEqual('');
});
it('sets the admin_user to false', function () {
Expand All @@ -81,7 +82,7 @@ describe('timeProfileFormController', function() {
});
});

describe('when the timeProfileFormId is an id', function() {
describe('when the timeProfileFormId is an id and action is timeprofile_edit', function() {
var timeProfileFormResponse = {
description: 'TimeProfileTest',
admin_user: true,
Expand All @@ -101,30 +102,86 @@ describe('timeProfileFormController', function() {
{
$scope: $scope,
timeProfileFormId: '12345',
timeProfileFormAction: 'timeprofile_edit',
miqService: miqService
});
$httpBackend.flush();
}));

it('sets the name to blank', function () {
it('sets formId to timeProfileFormId', function () {
expect($scope.vm.formId).toEqual('12345');
});
it('sets the description to correct value', function () {
expect($scope.vm.timeProfileModel.description).toEqual('TimeProfileTest');
});
it('sets the admin_user to true', function () {
expect($scope.vm.timeProfileModel.admin_user).toBeTruthy();
});
it('sets the restricted_time_profile to false', function () {
expect($scope.vm.timeProfileModel.restricted_time_profile).toBeFalsy();
});
it('sets the profile_type to correct value', function () {
expect($scope.vm.timeProfileModel.profile_type).toEqual('user');
});
it('sets the profile_tz to correct value', function () {
expect($scope.vm.timeProfileModel.profile_tz).toEqual('Alaska');
});
it('sets the all_days to true', function () {
expect($scope.vm.timeProfileModel.all_days).toBeTruthy();
});
it('sets the all_hours to true', function () {
expect($scope.vm.timeProfileModel.all_hours).toBeTruthy();
});
});

describe('when the timeProfileFormId is an id and action is timeprofile_copy', function() {
var timeProfileFormResponse = {
description: 'TimeProfileTest',
admin_user: true,
restricted_time_profile: false,
profile_type: 'user',
profile_tz: 'Alaska',
rollup_daily: true,
all_days: true,
all_hours: true,
days: [0, 1, 2, 3, 4, 5, 6],
hours: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23]
};

beforeEach(inject(function(_$controller_) {
$httpBackend.whenGET('/configuration/time_profile_form_fields/12345').respond(timeProfileFormResponse);
$controller = _$controller_('timeProfileFormController as vm',
{
$scope: $scope,
timeProfileFormId: '12345',
timeProfileFormAction: 'timeprofile_copy',
miqService: miqService
});
$httpBackend.flush();
}));

it('sets formId to new', function () {
expect($scope.vm.formId).toEqual('new');
});
it('sets the description to correct value', function () {
expect($scope.vm.timeProfileModel.description).toEqual('TimeProfileTest');
});
it('sets the admin_user to false', function () {
it('sets the admin_user to true', function () {
expect($scope.vm.timeProfileModel.admin_user).toBeTruthy();
});
it('sets the restricted_time_profile to false', function () {
expect($scope.vm.timeProfileModel.restricted_time_profile).toBeFalsy();
});
it('sets the profile_type to blank', function () {
it('sets the profile_type to correct value', function () {
expect($scope.vm.timeProfileModel.profile_type).toEqual('user');
});
it('sets the profile_tz to blank', function () {
it('sets the profile_tz to correct value', function () {
expect($scope.vm.timeProfileModel.profile_tz).toEqual('Alaska');
});
it('sets the all_days to false', function () {
it('sets the all_days to true', function () {
expect($scope.vm.timeProfileModel.all_days).toBeTruthy();
});
it('sets the all_hours to blank', function () {
it('sets the all_hours to true', function () {
expect($scope.vm.timeProfileModel.all_hours).toBeTruthy();
});
});
Expand Down