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

Upload documents to google drive (UD3, UD6.1, UD6.2) #1327

Merged
merged 4 commits into from
Feb 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
70 changes: 69 additions & 1 deletion app/controllers/hiring_staff/vacancies/documents_controller.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
require 'google/apis/drive_v3'

class HiringStaff::Vacancies::DocumentsController < HiringStaff::Vacancies::ApplicationController
before_action :school, :redirect_unless_vacancy_session_id, only: %i[index create]
before_action :redirect_if_no_supporting_documents, only: %i[index create]
before_action :redirect_to_next_step_if_save_and_continue, only: :create

def index
@documents_form = DocumentsForm.new
@vacancy = Vacancy.find(session[:vacancy_attributes]['id'])
end

def create
@documents_form = DocumentsForm.new(documents_form_params)
@documents_form = DocumentsForm.new
@vacancy = add_documents(process_documents(documents_form_params))
render :index
end

Expand All @@ -26,4 +30,68 @@ def redirect_if_no_supporting_documents
def redirect_to_next_step_if_save_and_continue
redirect_to application_details_school_job_path if params[:commit] == 'Save and continue'
end

def add_documents(documents_attributes)
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
vacancy ||= school.vacancies.find(session_vacancy_id)
documents_attributes.each do |document|
vacancy.documents.create(document)
end
vacancy
end

def process_documents(params)
@file_size_limit = 10 # MB
@errors = false
documents_array = []
if params[:documents]&.any?
params[:documents].each do |document_params|
document_hash = upload_document(document_params)
unless @errors
documents_array << document_hash
end
end
end
documents_array
end

def upload_document(document_params)
document_upload = DocumentUpload.new(
upload_path: document_params.tempfile.path,
name: document_params.original_filename
)
if document_params.size / 1024.0 / 1024.0 > @file_size_limit
file_size_error(document_params.original_filename)
end
unless @errors
document_upload.upload
unless document_upload.safe_download
virus_error(document_params.original_filename)
end
create_document_hash(document_params, document_upload)
end
end

def file_size_error(filename)
@errors = true
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there's a risk, in using a separate variable to track whether there are errors, that it will become decoupled from the actual state of @documents_form.errors which is partly determined by ActiveModel.

unless @errors 

could become (something like):

unless @documents_form.errors

in order to have a single source of truth about whether there are errors. Maybe this is just a pattern I'm unaware of though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, but I think this is because we don't validate whether we have a virus in the form model.
We wouldn't know if there was a virus unless Google tells us so the method is invoked depending on the response from Google.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the error is created on line 84:

@documents_form.errors.add(:base, t('jobs.file_virus_error_message', filename: filename))

Does that count as validating in the form model, for your purposes?

@documents_form.errors.add(
:base, t('jobs.file_size_error_message', filename: filename, size_limit: @file_size_limit)
)
@documents_form.errors.add(:documents, t('jobs.file_input_error_message'))
end

def virus_error(filename)
@errors = true
@documents_form.errors.add(:base, t('jobs.file_virus_error_message', filename: filename))
@documents_form.errors.add(:documents, t('jobs.file_input_error_message'))
end

def create_document_hash(params, upload)
{
name: params.original_filename,
size: Integer(params.size),
content_type: params.content_type,
download_url: upload.uploaded.web_content_link,
google_drive_id: upload.uploaded.id
}
end
end
4 changes: 3 additions & 1 deletion app/form_models/vacancy_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ class VacancyForm
delegate :save, to: :vacancy

def initialize(params = {})
@vacancy = Vacancy.new(params.except(:expiry_time_hh, :expiry_time_mm, :expiry_time_meridiem))
@vacancy = Vacancy.new(
params.except(:documents_attributes, :expiry_time_hh, :expiry_time_mm, :expiry_time_meridiem)
)
end

def school
Expand Down
2 changes: 1 addition & 1 deletion app/frontend/packs/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import 'src/removeCommaFromNumber';
import 'src/shareUrl';
import 'src/sortJobList';
import 'src/submitFeedback';
import 'src/vacancyShow';
import 'src/uploadDocuments';
import 'src/vacancyShow';

import { initAll } from 'govuk-frontend';

Expand Down
52 changes: 51 additions & 1 deletion app/frontend/src/uploadDocuments.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,68 @@ document.addEventListener('DOMContentLoaded', function() {
const inputFileUpload = document.getElementsByClassName('govuk-file-upload')[0];
const selectFileButton = document.getElementsByClassName('govuk-button--secondary')[0];
const uploadFileButton = document.getElementsByClassName('govuk-button--secondary')[1];
const saveContinueButton = document.getElementsByName('commit')[1];

selectFileButton.addEventListener('click', function(e) {
e.preventDefault();

inputFileUpload.click();
});

inputFileUpload.addEventListener('change', function(e) {
saveContinueButton.disabled = true;
injectDocumentsTable(inputFileUpload);
inputFileUpload.form.submit();
});

inputFileUpload.classList.add('display-none');
uploadFileButton.classList.add('display-none');
selectFileButton.classList.remove('display-none');
});

injectDocumentsTable = function(documentsInput) {
var tableHTML = " \
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
<table class='govuk-table'> \
<thead class='govuk-table__head'> \
<tr class='govuk-table__row'> \
<th class='govuk-table__header'>File name</> \
<th class='govuk-table__header'>Status</> \
<th class='govuk-table__header'>File size</> \
<th class='govuk-table__header'>Action</> \
</tr> \
</thead> \
<tbody id='table-body' class='govuk-table__body'> \
</tbody> \
</table> \
"
var p = document.getElementById('no-files');
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
var filesList = documentsInput.files;
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved

if (filesList.length) {
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
if (p) {
p.insertAdjacentHTML("beforebegin", tableHTML);
}
for (var i = 0; i < filesList.length; i++) {
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
var rowHTML = " \
<tr class='govuk-table__row'> \
<td class='govuk-table__cell' scope='row'>" +
filesList[i].name +
"</td> \
<td class='govuk-table__cell'>" +
"Uploading&nbsp;&nbsp;" + "<div class='upload-progress'><div></div><div></div><div></div><div></div></div>" +
"</td> \
<td class='govuk-table__cell' scope='row'>" +
(filesList[i].size / 1024.0 / 1024.0).toFixed(2) + " MB" +
"</td> \
<td class='govuk-table__cell' scope='row'>" +
"<a href='#' class='govuk-link govuk-link--no-visited-state'>Cancel</a>" +
"</td> \
</tr> \
"
var b = document.getElementById('table-body');
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
b.insertAdjacentHTML("beforeend", rowHTML);
}
if (p) {
p.remove();
}
}
}
38 changes: 37 additions & 1 deletion app/frontend/styles/components/upload-documents.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,43 @@
.upload-group .govuk-button {
#file-upload {
margin-bottom: 0;
}

.hidden-input {
display: none;
}

.upload-progress {
display: inline-block;
position: relative;
width: 18px;
height: 18px;
}
.upload-progress div {
box-sizing: border-box;
display: block;
position: absolute;
width: 18px;
height: 18px;
margin: 0px;
border: 3px solid green;
border-radius: 50%;
animation: lds-ring 1.2s cubic-bezier(0.5, 0, 0.5, 1) infinite;
border-color: green transparent transparent transparent;
}
.upload-progress div:nth-child(1) {
animation-delay: -0.45s;
}
.upload-progress div:nth-child(2) {
animation-delay: -0.3s;
}
.upload-progress div:nth-child(3) {
animation-delay: -0.15s;
}
@keyframes lds-ring {
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
4 changes: 4 additions & 0 deletions app/models/document.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class Document < ApplicationRecord
belongs_to :vacancy
validates :name, :size, :content_type, :download_url, :google_drive_id, presence: true
end
4 changes: 2 additions & 2 deletions app/models/vacancy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ class Vacancy < ApplicationRecord
extend FriendlyId
extend ArrayEnum

attr_accessor :documents

friendly_id :slug_candidates, use: %w[slugged history]

enum status: { published: 0, draft: 1, trashed: 2 }
Expand Down Expand Up @@ -122,6 +120,8 @@ class Vacancy < ApplicationRecord

has_one :publish_feedback, class_name: 'VacancyPublishFeedback'

has_many :documents

delegate :name, to: :school, prefix: true, allow_nil: false
delegate :geolocation, to: :school, prefix: true, allow_nil: true

Expand Down
52 changes: 52 additions & 0 deletions app/services/document_upload.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
require 'google/apis/drive_v3'

class DocumentUpload
class MissingUploadPath < StandardError; end
attr_accessor :drive_service, :upload_path, :name, :uploaded, :safe_download

def initialize(opts = {})
raise MissingUploadPath if opts[:upload_path].nil?
self.upload_path = opts[:upload_path]
self.name = opts[:name]
self.drive_service = Google::Apis::DriveV3::DriveService.new
end

def upload
upload_hiring_staff_document
set_public_permission_on_document
google_drive_virus_check
end

def upload_hiring_staff_document
self.uploaded = drive_service.create_file(
{ alt: 'media', name: name },
fields: 'id, web_view_link, web_content_link, mime_type',
upload_source: upload_path
)
end

def set_public_permission_on_document
drive_service.create_permission(
uploaded.id,
Google::Apis::DriveV3::Permission.new(type: 'anyone', role: 'reader')
)
end

def google_drive_virus_check
download_path = "#{uploaded.id}"
begin
drive_service.get_file(
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
uploaded.id,
acknowledge_abuse: false,
download_dest: download_path
)
rescue Google::Apis::ClientError
drive_service.delete_file(uploaded.id)
self.safe_download = false
else
self.safe_download = true
ensure
File.delete(download_path) if File.exist?(download_path)
end
end
end
20 changes: 11 additions & 9 deletions app/views/hiring_staff/vacancies/documents/_documents.html.haml
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
%table.govuk-table
%thead.govuk-table__head
%tr.govuk-table__row
%th.govuk-table__header File name
%th.govuk-table__header Status
%th.govuk-table__header File size
%th.govuk-table__header Action
%tbody.govuk-table__body
%th.govuk-table__header= t('jobs.upload_documents_table.headers.file_name')
%th.govuk-table__header= t('jobs.upload_documents_table.headers.status')
%th.govuk-table__header= t('jobs.upload_documents_table.headers.file_size')
%th.govuk-table__header= t('jobs.upload_documents_table.headers.action')
%tbody#table-body.govuk-table__body
- documents.each do |document|
%tr.govuk-table__row
%th.govuk-table__header{ scope: 'row' } #{document.original_filename}
%td.govuk-table__cell{ scope: 'row' }
%a.govuk-link{ href: document[:download_url], target: '_blank' } #{document[:name]}
%td.govuk-table__cell= t('upload_documents_table.upload_status.uploaded')
%td.govuk-table__cell{ scope: 'row' } #{number_with_precision(document[:size] / 1024.0 / 1024.0, precision: 2)} MB
%td.govuk-table__cell
%td.govuk-table__cell
%td.govuk-table__cell
%a{ href: '#' } Remove
%a.govuk-link.govuk-link--no-visited-state{ href: '#' }
= t('upload_documents_table.actions.remove')
8 changes: 4 additions & 4 deletions app/views/hiring_staff/vacancies/documents/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

.govuk-grid-row
.govuk-grid-column-two-thirds
= form_for @documents_form, url: documents_school_job_path(school_id: @school.id), builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f|
= form_for @documents_form, url: documents_school_job_path, builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f|
= f.govuk_error_summary 'Please correct the following errors'

%h2.govuk-heading-m
Expand All @@ -23,9 +23,9 @@

= f.govuk_submit t('jobs.upload_file'), secondary: true

- if @documents_form.vacancy.documents&.any?
= render 'hiring_staff/vacancies/documents/documents', documents: @documents_form.vacancy.documents
- if @vacancy&.documents&.any?
= render 'hiring_staff/vacancies/documents/documents', documents: @vacancy.documents
- else
%p= t('jobs.no_files_message')
%p#no-files= t('jobs.no_files_message')

= f.govuk_submit t('buttons.save_and_continue')
42 changes: 42 additions & 0 deletions app/views/hiring_staff/vacancies/documents/new.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
%h1.govuk-heading-l
cpjmcquillan marked this conversation as resolved.
Show resolved Hide resolved
= t('jobs.publish_heading', school: @school.name)
%span.govuk-caption-l
Step 2 of 3

.gov-uk-main-wrapper
.govuk-grid-row
.govuk-grid-column-two-thirds
%form{action: "#", method: "post", enctype: "multipart/form-data"}
%h2.govuk-heading-m
= t('jobs.supporting_documents')
.govuk-form-group.upload-group
%fieldset.govuk-fieldset{"aria-describedby": "upload-supporting-documents"}
%label.govuk-label.govuk-label--s{class: "govuk-!-margin-bottom-3", for: "file-upload"}
= t('jobs.upload_file')
%button#file-upload.govuk-button.govuk-button--secondary{"data-module": "govuk-button"}
= t('jobs.choose_file')
%input#upload.hidden-input{type: "file", name: "upload", onchange: "form.submit()"}
%table.govuk-table
%thead.govuk-table__head
%tr.govuk-table__row
%th.govuk-table__header
= t('jobs.upload_documents_table.headers.file_name')
%th.govuk-table__header
= t('jobs.upload_documents_table.headers.status')
%th.govuk-table__header
= t('jobs.upload_documents_table.headers.file_size')
%th.govuk-table__header
= t('jobs.upload_documents_table.headers.action')
%tbody.govuk-table__body
%tr.govuk-table__row
%td.govuk-table__cell
File name
%td.govuk-table__cell
Uploaded
%td.govuk-table__cell
24.5 MB
%td.govuk-table__cell
%a.govuk-link{href: '#'}
remove
%a.govuk-button{href: "#"}
= t('buttons.save_and_continue')
Loading