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

148/scaffold/Complete VM to responsable relation #284

Merged
merged 35 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d4c0b52
Associate requests with responsible users
LeonMatthes Jan 21, 2019
c985ec3
Refactorings
LeonMatthes Jan 21, 2019
28cf805
Merge remote-tracking branch 'origin/dev' into 148/scaffold/VM-to-res…
LeonMatthes Jan 23, 2019
12e7510
Test responsible user access in VSphere::VirtualMachine
LeonMatthes Jan 23, 2019
d05f1af
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
regnujAx Jan 27, 2019
7c56fdc
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
regnujAx Jan 29, 2019
dc63d5c
try to fix some issues of CodeFactor
regnujAx Jan 29, 2019
ccd24c3
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
regnujAx Jan 31, 2019
159c218
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
LeonMatthes Jan 31, 2019
1347f8c
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
regnujAx Jan 31, 2019
b65ea75
Merge remote-tracking branch 'origin/dev' into 148/scaffold/VM-to-res…
LeonMatthes Feb 1, 2019
df3a542
Refactoring
LeonMatthes Feb 1, 2019
562a555
Fix tests
LeonMatthes Feb 1, 2019
f256541
Add responsible users to VMConfigs
LeonMatthes Feb 1, 2019
2ed7955
More tests
LeonMatthes Feb 1, 2019
a259c00
Fix potential issue, if a vm is called requests or configs
LeonMatthes Feb 1, 2019
da09d61
Add tests for vm creation
LeonMatthes Feb 1, 2019
1d480c6
Responsible users are now also stored in the VM config
LeonMatthes Feb 1, 2019
5ab9ebe
responsible users are now assigned to virtual machines
LeonMatthes Feb 1, 2019
9f1288c
Add responsible to notificaiton
LeonMatthes Feb 1, 2019
dcb34cb
Work on moving into correct subfolder
LeonMatthes Feb 1, 2019
4f4d5fa
VM now moves into the correct subfolder whenever its created, archive…
LeonMatthes Feb 1, 2019
3e3cf59
Move responsibilty for VM creation from folder to request
LeonMatthes Feb 1, 2019
b51c834
Whoops
LeonMatthes Feb 1, 2019
59228fb
Add test for move_into_correct_subfolder
LeonMatthes Feb 1, 2019
a68be80
Merge remote-tracking branch 'origin/dev' into 148/scaffold/VM-to-res…
LeonMatthes Feb 1, 2019
11deaa0
Migrate
LeonMatthes Feb 1, 2019
075fad3
Fix end to end testing
LeonMatthes Feb 1, 2019
cc4a985
Merge branch 'dev' into 148/scaffold/VM-to-responsible-relation
LeonMatthes Feb 1, 2019
6caf79d
Fix some CodeFactor issues
LeonMatthes Feb 1, 2019
1b846bb
Merge branch '148/scaffold/VM-to-responsible-relation' of https://git…
LeonMatthes Feb 1, 2019
71c8bd0
Remove unneccessary method
LeonMatthes Feb 1, 2019
45fd8ae
Comment routes to be clearer why the change is neccessary
LeonMatthes Feb 1, 2019
5825d42
Make CodeFactor a bit happy
LeonMatthes Feb 1, 2019
a0272ab
Fix in skipped test
LeonMatthes Feb 1, 2019
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
16 changes: 8 additions & 8 deletions app/api/v_sphere/virtual_machine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ def boot_time
@vm.runtime.bootTime
end

def responsible_users
request&.responsible_users || []
end

def users
request&.users || []
end

def summary
@vm.summary
end
Expand Down Expand Up @@ -229,14 +237,6 @@ def status
end
end

def users
if request
request.users
else
[]
end
end

def sudo_users
if request
request.sudo_users
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,10 @@ def prepare_params

# Never trust parameters from the scary internet, only allow the white list through.
def request_params
params[:request][:name] = replace_whitespaces(params[:request][:name]) if params[:request] && params[:request][:name]
params.require(:request).permit(:name, :cpu_cores, :ram_mb, :storage_mb, :operating_system,
:port, :application_name, :description, :comment,
:rejection_information, user_ids: [], sudo_user_ids: [])
:rejection_information, responsible_user_ids: [], user_ids: [], sudo_user_ids: [])
end

def rejection_params
Expand Down
3 changes: 2 additions & 1 deletion app/models/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Request < ApplicationRecord
has_many :users_assigned_to_requests
has_many :users, through: :users_assigned_to_requests
belongs_to :user
has_and_belongs_to_many :responsible_users, class_name: 'User', join_table: 'requests_responsible_users'

attr_accessor :sudo_user_ids

Expand All @@ -17,7 +18,7 @@ class Request < ApplicationRecord
length: { maximum: MAX_NAME_LENGTH, message: 'only allows a maximum of %{count} characters' },
format: { with: /\A[a-zA-Z1-9\-\s]+\z/, message: 'only letters and numbers allowed' },
uniqueness: true
validates :cpu_cores, :ram_mb, :storage_mb, :operating_system, :description, presence: true
validates :responsible_users, :cpu_cores, :ram_mb, :storage_mb, :operating_system, :description, presence: true
validates :cpu_cores, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_CPU_CORES }
validates :ram_mb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_RAM_MB }
validates :storage_mb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_STORAGE_MB }
Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class User < ApplicationRecord
has_many :users_assigned_to_requests
has_many :requests, through: :users_assigned_to_requests
has_many :servers
has_and_belongs_to_many :request_responsibilities, class_name: 'Request', join_table: 'requests_responsible_users'
validates :first_name, presence: true
validates :last_name, presence: true
validate :valid_ssh_key
Expand Down
3 changes: 2 additions & 1 deletion app/views/errors/timeout.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<h1 class="display-1">Sorry!</h1>
<h2 class="lead">
It seems you have lost connection to the HPI network.
We could not reach vSphere.<br/>
Please make sure you are connected to the HPI network.
</h2>
10 changes: 10 additions & 0 deletions app/views/requests/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
<%= form.text_field :name, class: 'form-control' %>
</div>

<div class="field form-group">
<p>Responsible Users</p>
<%= form.select 'responsible_user_ids',
options_from_collection_for_select(User.all,
:id,
:email),
{},
{ multiple: true, class: "selecttwo", data: { placeholder: 'Who is responsible for this VM?' }, style: 'width: 50%' } %>
</div>

<div class="field form-group">
<%= form.label :cpu_cores, "CPU cores" %>
<%= form.number_field :cpu_cores, min: 0, class: 'form-control', id: 'cpu' %>
Expand Down
33 changes: 21 additions & 12 deletions app/views/requests/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,57 +3,66 @@
<table class="table table-fixed-columns table-flex-width">
<tbody>
<tr>
<th style="width: 300px">VM Name:</td>
<th style="width: 300px">VM Name</td>
<td><%= @request.name %></td>
</tr>

<tr>
<td><strong>CPU cores:</strong></td>
<td><strong>CPU cores</strong></td>
<td><%= @request.cpu_cores %></td>
</tr>

<tr>
<td><strong>RAM in GB:</strong></td>
<td><strong>RAM in GB</strong></td>
<td><%= mb_to_gb(@request.ram_mb) %></td>
</tr>

<tr>
<td><strong>Storage in GB:</strong></td>
<td><strong>Storage in GB</strong></td>
<td><%= mb_to_gb(@request.storage_mb) %></td>
</tr>

<tr>
<td><strong>Operating System:</strong></td>
<td><strong>Operating System</strong></td>
<td><%= @request.operating_system %></td>
</tr>

<tr>
<td><strong>Port:</strong></td>
<td><strong>Port</strong></td>
<td><%= @request.port %></td>
</tr>

<tr>
<td><strong>Application Name:</strong></td>
<td><strong>Application Name</strong></td>
<td><%= @request.application_name %></td>
</tr>

<tr>
<td><strong>Description:</strong></td>
<td><strong>Description</strong></td>
<td class="col-md-4"><%= @request.description %></td>
</tr>

<tr>
<td><strong>Comment:</strong></td>
<td><strong>Comment</strong></td>
<td class="col-md-4"><%= @request.comment %></td>
</tr>

<tr>
<td><strong>Status:</strong></td>
<td><strong>Status</strong></td>
<td><%= @request.status %></td>
</tr>

<tr>
<td><strong>Users with sudo rights:</strong></td>
<td><strong>Responsible users</strong></td>
<td>
<% @request.responsible_users.each do |user| %>
<p><%= user.email %></p>
<% end %>
</td>
</tr>

<tr>
<td><strong>Users with sudo rights</strong></td>
<td>
<% @request.sudo_user_assignments.each do |assignment| %>
<p><%= assignment.user.email %></p>
Expand All @@ -62,7 +71,7 @@
</tr>

<tr>
<td><strong>Users:</strong></td>
<td><strong>Users</strong></td>
<td>
<% @request.non_sudo_user_assignments.each do |assignment| %>
<p><%= assignment.user.email %></p>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class CreateJoinTableRequestsResponsibleUsers < ActiveRecord::Migration[5.2]
def change
create_join_table(:requests, :users, table_name: 'requests_responsible_users') do |t|
t.index [:request_id, :user_id]
end
end
end
15 changes: 13 additions & 2 deletions spec/api/v_sphere_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,26 @@
expect(VSphere::VirtualMachine.find_by_name('Archived VM2')).not_to be_nil
end

it 'does not have users if there is not fitting request' do
it 'does not have users if there is no fitting request' do
expect(VSphere::VirtualMachine.find_by_name('Archived VM2').users).to be_empty
end

it 'has users if a fitting request exists' do
request = FactoryBot.create :accepted_request
request.users << FactoryBot.create(:user)
vm = VSphere::VirtualMachine.new(vim_vm_mock(request.name))
expect(vm.users).not_to be_empty
expect(vm.users).to match_array(request.users)
end

it 'does not have responsible users if there is no fitting request' do
expect(VSphere::VirtualMachine.find_by_name('Archived VM2').responsible_users).to be_empty
end

it 'has responsible users if a fitting request exists' do
request = FactoryBot.create :accepted_request
request.responsible_users << FactoryBot.create(:user)
vm = v_sphere_vm_mock request.name
expect(vm.responsible_users).to match_array(request.responsible_users)
end

it 'does not have an IP if the fitting config does not exist' do
Expand Down
12 changes: 9 additions & 3 deletions spec/controllers/requests_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
comment: 'Comment',
status: 'pending',
user: user,
sudo_user_ids: ['']
sudo_user_ids: [''],
responsible_user_ids: [user.id]
}
end

Expand Down Expand Up @@ -118,21 +119,26 @@
end

context 'with invalid params' do
before do
post :create, params: { request: invalid_attributes }
it 'does not create a request if responible users are empty' do
request_count = Request.all.size
post :create, params: { request: valid_attributes.except(:responsible_user_ids) }
expect(request_count).to equal(Request.all.size)
end

it 'returns a success response (i.e. to display the "new" template)' do
post :create, params: { request: invalid_attributes }
expect(response).to be_successful
end

# regression test for #320
it 'assigns the sudo users to the request' do
post :create, params: { request: invalid_attributes }
expect(assigns(:request).sudo_users).to match_array([sudo_user])
end

# regression test for #320
it 'does not persist the sudo users' do
post :create, params: { request: invalid_attributes }
assigns(:request).users_assigned_to_requests.each do |each|
expect(each).not_to be_persisted
end
Expand Down
1 change: 1 addition & 0 deletions spec/factories/requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
comment { 'Comment' }
status { 'pending' }
user { FactoryBot.create :admin }
responsible_users { [FactoryBot.create(:user)] }
end

factory :rejected_request, parent: :request do
Expand Down
10 changes: 0 additions & 10 deletions spec/features/dashboard/dashboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,6 @@
it 'does render a list of all available vms for a signed in user' do
visit(:dashboard)
expect(page).to(have_text('VMs'))
skip('user is not yet connected to his vms')
end

it 'does render an empty list if a user does not have access to a vm' do
skip('user is not yet connected to his vms')
end

it 'does render a selection of details per vm' do
skip('user is not yet connected to his vms')
end

it 'does render notifications for a user' do
Expand All @@ -30,7 +21,6 @@
it 'does render a list of servers for a signed user' do
visit(:dashboard)
expect(page).to(have_text('Hosts'))
skip('user is not yet connected to his hosts')
end

context 'with notifications' do
Expand Down
3 changes: 2 additions & 1 deletion spec/views/requests/edit.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
description: 'Description',
comment: 'Comment',
status: 'pending',
user: FactoryBot.create(:employee)
user: FactoryBot.create(:employee),
responsible_users: [FactoryBot.create(:user)]
))
assign(:request_templates, [RequestTemplate.new(
name: 'MyTemplate',
Expand Down
6 changes: 4 additions & 2 deletions spec/views/requests/index.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
description: 'Description',
comment: 'Comment',
status: 'pending',
user: FactoryBot.create(:employee)
user: FactoryBot.create(:employee),
responsible_users: [FactoryBot.create(:user)]
),
Request.create!(
name: 'MyVM1',
Expand All @@ -29,7 +30,8 @@
description: 'Description',
comment: 'Comment',
status: 'pending',
user: FactoryBot.create(:employee)
user: FactoryBot.create(:employee),
responsible_users: [FactoryBot.create(:user)]
)
]
end
Expand Down
3 changes: 2 additions & 1 deletion spec/views/requests/show.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
comment: 'Comment',
status: 'pending',
user_ids: [@user.id],
user: FactoryBot.create(:employee)
user: FactoryBot.create(:employee),
responsible_users: [FactoryBot.create(:user)]
))
@request.assign_sudo_users([@second_user.id])
allow(view).to receive(:current_user).and_return(current_user)
Expand Down