Upload method for 'Errors Found and Corrected' #23607
Conversation
| end | ||
|
|
||
| def error_found_upload_transcription_to_vbms; end | ||
| def error_found_upload_transcription_to_vbms |
There was a problem hiding this comment.
Should refactor to keep much of this logic abstracted out of the tasks_controller - much of whats going on here is not the responsibility of the tasks_controller. Method length is too long. Ruby methods should be 10 lines or less. Take a look at TranscriptionFileUpload class.
app/controllers/tasks_controller.rb
Outdated
| S3Service.store_file(s3_location(file_name), tmp_folder, :filepath) | ||
| rescue StandardError => error | ||
| Rails.logger.error "Error to upload #{file_name} to S3: #{error.message}" | ||
| raise BoxDownloadJobFileUploadError |
There was a problem hiding this comment.
Sounds like incorrect error class - this upload doesn't seem related to a BoxDownloadJob
app/controllers/tasks_controller.rb
Outdated
| end | ||
| end | ||
|
|
||
| def s3_location(file_name) |
There was a problem hiding this comment.
This sounds like the responsibility of TranscriptionFileUpload
app/controllers/tasks_controller.rb
Outdated
|
|
||
| private | ||
|
|
||
| def split_filename(filename) |
There was a problem hiding this comment.
I think we will want "file_name" to be consistent throughout this file, rather than having both "filename" and "file_name"
app/controllers/tasks_controller.rb
Outdated
| updated_at: Time.zone.now | ||
| ) | ||
|
|
||
| Task.where(id: task.id).update_all( |
There was a problem hiding this comment.
task.update!...
We already have the task - no need to call where which returns an array - and update_all would imply there are multiples. Since we're querying on id, there can only be one.
app/controllers/tasks_controller.rb
Outdated
| } | ||
| end | ||
|
|
||
| def change_status_review_transcript_task |
There was a problem hiding this comment.
This method name should reflect exactly whats going on - perhaps complete_transcript_review. Since this is updating the status in a specific way, and also updating transcriptions.
app/controllers/tasks_controller.rb
Outdated
| @@ -51,6 +51,8 @@ class TasksController < ApplicationController | |||
| SplitAppealTask: SplitAppealTask | |||
| }.freeze | |||
|
|
|||
| S3_BUCKET = "vaec-appeals-caseflow" | |||
There was a problem hiding this comment.
this isn't being used here anymore and can be removed
app/controllers/tasks_controller.rb
Outdated
| ) | ||
|
|
||
| render json: { | ||
| tasks: json_tasks(task.appeal.tasks.includes(*task_includes))[:data] |
There was a problem hiding this comment.
I don't believe this is the correct json response we want to return after this call. json_tasks is meant to populate a queue table and is a large response with a lot of data. I don't see how the front end would be using any data from this response other than an ok status.
Resolves APPEALS-64078
Description
As a Caseflow User I need to be able to click the submit button to upload the "final PDF" to the correct Appellants eFolder in VBMS so that the final PDF Transcript is uploaded Appellants eFolder in VBMS.
As a Caseflow Developer I need to;
DocketNumber_interenalHearingID_HearingClassName.pdf
Acceptance Criteria
Frontend
Hearing Admin Team casesErrors found and correctedUpload to VBMSTests
Test Coverage
Did you include any test coverage for your code? Check below: