-
Notifications
You must be signed in to change notification settings - Fork 44
SAT-35237 - Keep report in generated folder for disconnected users #1050
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
base: develop
Are you sure you want to change the base?
Conversation
Reviewer's GuideEnhance QueueForUploadJob to respect a disconnected flag or disabled subscription setting by skipping report movement and upload with a standardized log message, propagate organization_id for context, and introduce comprehensive unit tests covering both connected and disconnected flows. Sequence diagram for QueueForUploadJob with disconnected modesequenceDiagram
participant User as actor User
participant Job as QueueForUploadJob
participant Logger as Logger
participant Org as Organization
participant Setting as Setting
User->>Job: plan(base_folder, report_file, organization_id, disconnected)
Job->>Setting: Check subscription_connection_enabled
alt Disconnected mode (disconnected flag or setting false)
Job->>Org: Find organization by id
Job->>Logger: log_disconnected_message(report_file, organization_id)
Job-->>User: Return (report remains in generated folder)
else Connected mode
Job->>Job: plan_upload_report(enqueued_file_name, organization_id, disconnected)
Job-->>User: Continue with upload
end
Class diagram for updated QueueForUploadJobclassDiagram
class QueueForUploadJob {
+plan(base_folder, report_file, organization_id, disconnected)
+run()
-log_disconnected_message(report_file, organization_id)
+report_file
+organization_id
+content_disconnected?
+plan_upload_report(enqueued_file_name, organization_id, disconnected)
<<EntryAction>>
DISCONNECTED_MESSAGE_TEMPLATE
}
class UploadReportJob
class Organization {
+find_by(id)
+name
}
class Setting
QueueForUploadJob --> UploadReportJob : plans
QueueForUploadJob --> Organization : uses
QueueForUploadJob --> Setting : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
054b34f
to
3459988
Compare
* Report was being moved regardless of the setting so added logger statements and to return out of the task if the disconnected setting is on. * Added unit tests for queue_for_upload_job since we didn't have any and added some for the change I made
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @chris1984 - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
What are the changes introduced in this pull request?
Considerations taken when implementing this change?
What are the testing steps for this pull request?
subscription_connection_enabled
to falseuploads
generated
15:04:54 rails.1 | 2025-07-30T15:04:54 [I|bac|aaed2ff0] Report not moved because connection to Insights is not enabled or the --no-upload option was passed. Report: report_for_1.tar.xz, Organization: Default Organization
Summary by Sourcery
Handle disconnected mode in QueueForUploadJob by skipping report upload and retaining files in the generated folder, while logging contextual messages, and add comprehensive tests for these behaviors
New Features:
Enhancements:
Tests: