feat: Add preview to workflow execution files #949
Conversation
joshsadam
force-pushed
the
preview-pipeline-result-files
branch
4 times, most recently
from
3d0be02 to
ad1087c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Code Metrics Report
Code coverage of files in pull request scope (90.8%)Reported by octocov |
There was a problem hiding this comment.
Handful of comments to start, apologies if you were already working through them.
Your previews from the seeded files work fine, but I ran a iridanextexample pipeline and now have this error if I try to preview any attached file format (fasta or text) associated with the workflow.
| @@ -14,6 +14,7 @@ class Attachment < ApplicationRecord | |||
| 'spreadsheet' => /^\S+\.(xls|xlsx)?$/, | |||
| 'json' => /^\S+\.(json)?(\.gz)?$/, | |||
| 'genbank' => /^\S+\.(gbk|gbf|genbank)?(\.gz)?$/, | |||
| 'image' => /^\S+\.(png|jpg|jpeg|gif|bmp|tiff|svg|webp)$/, | |||
There was a problem hiding this comment.
Could you add a pill color for image in pill_color_helper.rb
There was a problem hiding this comment.
Added in 4a741f2
| <%= link_to workflow_executions_attachments_path(attachment: attachment.id, workflow_execution: @workflow_execution.id), class: "text-blue-600 dark:text-blue-100 font-semibold hover:underline" do %> | ||
| <%= t(".preview") %> | ||
| <% end %> |
There was a problem hiding this comment.
I was thinking maybe we just hide any preview links for formats that are not previewable, but figure you don't want to confuse users with seemingly 'random' missing preview links?
There was a problem hiding this comment.
I actually like the idea of not showing the button. I simplified some of the logic by moving some functionality onto the attachment model. 0f170ab
| @@ -75,4 +78,25 @@ | |||
| <td class="px-6 py-4"> | |||
| <%= local_time_ago(attachment.created_at) %> | |||
| </td> | |||
|
|
|||
| <td class="flex justify-end px-6 py-4 space-x-2"> | |||
| <%= link_to workflow_executions_attachments_path(attachment: attachment.id, workflow_execution: @workflow_execution.id), class: "text-blue-600 dark:text-blue-100 font-semibold hover:underline" do %> | |||
There was a problem hiding this comment.
Can the font be updated to use font-medium as that's more in line with the rest of the action links for the website
There was a problem hiding this comment.
Updated to match styles on workflow executions and data exports table. Thanks for the catch f35670a
| class: "text-blue-900 dark:text-blue-100 font-semibold hover:underline" %> | ||
| <% else %> | ||
| <%= link_to t(".download"), | ||
| rails_blob_path(attachment.file), | ||
| data: { | ||
| turbo: false, | ||
| }, | ||
| class: "text-blue-600 dark:text-blue-100 font-semibold hover:underline" %> |
There was a problem hiding this comment.
Can the font be updated to use font-medium as that's more in line with the rest of the action links for the website
There was a problem hiding this comment.
Updated to match styles on workflow executions and data exports table. Thanks for the catch f35670a
| def context_crumbs | ||
| @context_crumbs = [base_crumb].then do |crumbs| | ||
| @workflow_execution ? crumbs.concat(workflow_execution_crumbs) : crumbs | ||
| end | ||
| end |
There was a problem hiding this comment.
Could this be simplified to just use:
def context_crumbs
super
@context_crumbs += workflow_execution_crumbs
end
There was a problem hiding this comment.
You're smarter than me ;), updated in d7d7a15
config/locales/fr.yml
Outdated
| create: | ||
| error_message: 'The following errors prevented the Workflow Execution from being created:' | ||
| error_message: 'Les erreurs suivantes ont empêché la création de l''exécution du pipeline:' |
There was a problem hiding this comment.
Vous avez un typo ici (l''execution).
There was a problem hiding this comment.
Maudit copilote de GitHub, je ne peux même pas traduire correctement 😂, updated in 132ff4f
- Add new "Actions" column to workflow execution files table - Implement preview and download buttons for attachments - Update localization files with new translation keys for actions - Improve table layout and styling for better user interaction
- Standardize string value quotes in en.yml and fr.yml to single quotes - Maintain consistent formatting across localization files - Improve readability and consistency of translation files
- Add new "Actions" column to workflow execution files table - Implement preview and download buttons for attachments - Update localization files with new translation keys for actions - Improve table layout and styling for better user interaction
- Standardize string value quotes in en.yml and fr.yml to single quotes - Maintain consistent formatting across localization files - Improve readability and consistency of translation files
- Implement new AttachmentsController for handling attachment previews - Create dedicated view for attachment preview with file details and download option - Add route for attachment preview in workflow executions - Support preview for image and text file types - Update attachment partial to link to new preview page
- Refactor clipboard controller to provide better copy interaction - Add visual feedback for successful text copying - Implement clipboard button for text attachments with success state - Update attachment preview view to support clipboard interaction
- Add localization keys for copy, copied, and download actions - Replace hardcoded text with translation method calls - Update both English and French locale files - Improve code readability and maintainability
- Add localization keys for file preview unavailability - Update view to use translation method for preview messages - Adjust icon size for better visual presentation - Update both English and French locale files
- Enhance file preview with line numbers for better readability - Implement line-by-line rendering with sequential numbering - Style line numbers with subtle visual separation
- Enhance AttachmentsController with breadcrumb navigation - Add workflow execution context to attachment preview route - Update attachment partial to include workflow execution context - Refactor context crumbs generation for better navigation
- Add workflow execution ID to attachment preview route - Ensure proper navigation context when previewing attachments - Maintain existing styling and functionality
- Add error handling for non-existent attachments - Render 404 page when workflow execution is not found - Update context crumbs to include 'files' tab for better navigation - Enhance user experience with clear error messaging
- Enhance context crumbs generation to handle nil workflow execution - Simplify breadcrumb generation logic - Remove redundant error handling in workflow_execution method - Ensure robust navigation context for attachments preview
- Extract base and workflow execution crumbs into separate methods - Improve code organization and readability - Simplify context_crumbs method with method extraction - Maintain existing breadcrumb navigation logic
…and add missing locale entry
…hment rendering and error handling
…types and content properties
…ions attachment preview
…in attachments controller tests
…for CSV, JSON, text, and TSV formats
…line file attachment process in seed data
…on and preview capabilities
…result length and data
Introduces methods to determine if an attachment can be previewed or copied based on its file format. Defines constants PREVIEWABLE_TYPES and COPYABLE_TYPES to support these methods, with comprehensive test coverage for various file types.
Removes the FileFormatHandler concern and its corresponding test file, likely due to the implementation of similar functionality in the Attachment model in a previous commit.
Updates the workflow execution attachments views and controller to leverage new Attachment model methods for determining preview and copy capabilities. Removes redundant preview type determination logic and uses model-level methods instead.
…ttachmentsController Updates the context_crumbs method to use super and append workflow execution crumbs, reducing redundant code and improving method clarity.
…ution error message
joshsadam
force-pushed
the
preview-pipeline-result-files
branch
from
132ff4f to
3d6881f
Compare
I couldn't get the demo pipeline running tonight, I am off tomorrow but I will figure it out on Monday. |
What does this PR do and why?
Describe in detail what your merge request does and why.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other pull requests.
Text:
JSON:
CSV:
TSV:
Excel:
Images:
If the file type is not supported:
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
PR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.