Draft: Instance Integrations test of shared module code
What does this MR do and why?
Mentions #499482 (closed)
[1] pry(main)> Integrations::Instance::Jenkins.create!
=> #<Integrations::Instance::Jenkins:0x00000001561f2f28
[4] pry(main)> Integrations::Jenkins.create!(group: Group.last)
=> #<Integrations::Jenkins:0x0000000137b121e0
[1] pry(main)> Integrations::Jira.create!(group: Group.last)
=> #<Integrations::Jira:0x00000001636b5a08
[2] pry(main)> Integrations::Instance::Jira.create!
=> #<Integrations::Instance::Jira:0x0000000162812a00
[4] pry(main)> Integrations::Instance::Jira.last.jira_tracker_data
=> #<Integrations::JiraTrackerData:0x000000015ac31cd8
id: 12,
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Merge request reports
Activity
assigned to @georgekoltsov
added pipelinetier-1 label
- A deleted user
added backend database databasereview pending labels
7 7 8 8 self.allow_legacy_sti_class = true 9 9 10 belongs_to :integration 11 validates :integration, presence: true 10 belongs_to :integration, optional: true 11 belongs_to :instance_integration, optional: true 12 13 validates :integration, presence: true, if: :integration Did you consider new validations can break existing records? Please follow the code quality guidelines about new model validations when adding a new model validation.
If you're adding the validations to a model with no records you can ignore this warning.
7 7 8 8 self.allow_legacy_sti_class = true 9 9 10 belongs_to :integration 11 validates :integration, presence: true 10 belongs_to :integration, optional: true 11 belongs_to :instance_integration, optional: true 12 13 validates :integration, presence: true, if: :integration 14 validates :instance_integration, presence: true, if: :instance_integration Did you consider new validations can break existing records? Please follow the code quality guidelines about new model validations when adding a new model validation.
If you're adding the validations to a model with no records you can ignore this warning.
1 Error The database migration pipeline
must be triggered by the jobdb:gitlabcom-database-testing
must be run before requesting
database review. This job takes ~30m and will post the results to a comment on the merge
request. Please run and review any results before passing to a reviewer.8 Warnings This merge request is definitely too big (4064 lines changed), please split it into multiple merge requests. b0ee2114: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. b0ee2114: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. b0ee2114: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. 1aa7f5a6: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. This merge request does not refer to an existing milestone. You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the maintenancepipelines, maintenancerefactor, maintenanceworkflow, documentation, QA labels.app/models/integrations/base_ci.rb
was removed but is mentioned in:.rubocop_todo/layout/class_structure.yml:75
.rubocop_todo/style/redundant_self.yml:79
2 Messages CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelog
trailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20241018131433_update_instance_integration_table.rb
db/migrate/20241018143504_add_instance_integration_fk_to_web_hooks.rb
db/migrate/20241018152911_add_foreign_key_to_web_hooks_instance_integration_id.rb
db/migrate/20241018163311_add_instance_integration_column_to_jira_tracker_data.rb
db/migrate/20241018163437_add_instance_integration_f_key_to_jira_tracker_data.rb
db/migrate/20241018163848_remove_not_null_constraint_jira_tracker_data.rb
db/schema_migrations/20241018131433
db/schema_migrations/20241018143504
db/schema_migrations/20241018152911
db/schema_migrations/20241018163311
db/schema_migrations/20241018163437
db/schema_migrations/20241018163848
db/docs/instance_integrations.yml
db/structure.sql
Reviewer roulette
Category Reviewer Maintainer backend @ddieulivol
(UTC+2, 1 hour ahead of author)
@dskim_gitlab
(UTC+11, 10 hours ahead of author)
database @suraj_tripathy
(UTC+5.5, 4.5 hours ahead of author)
@Quintasan
(UTC+2, 1 hour ahead of author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Danger- A deleted user
added Data WarehouseImpact Check label
@georgekoltsov
- please see the following guidance and update this merge request.1 Error Please add typebug typefeature, or typemaintenance label to this merge request.
added 1 commit
- 205d66af - Instance Integrations test of shared module code
added 1 commit
- 1aa7f5a6 - Instance Integrations test of shared module code
6 6 7 7 included do 8 8 belongs_to :integration, inverse_of: self.table_name.to_sym, foreign_key: :integration_id 9 belongs_to :instance_integration, inverse_of: self.table_name.to_sym, foreign_key: :instance_integration_id, class_name: 'Integrations::Instance::Integration' 9 10 10 validates :integration, presence: true 11 validates :integration, presence: true, if: :integration Did you consider new validations can break existing records? Please follow the code quality guidelines about new model validations when adding a new model validation.
If you're adding the validations to a model with no records you can ignore this warning.
6 6 7 7 included do 8 8 belongs_to :integration, inverse_of: self.table_name.to_sym, foreign_key: :integration_id 9 belongs_to :instance_integration, inverse_of: self.table_name.to_sym, foreign_key: :instance_integration_id, class_name: 'Integrations::Instance::Integration' 9 10 10 validates :integration, presence: true 11 validates :integration, presence: true, if: :integration 12 validates :instance_integration, presence: true, if: :instance_integration Did you consider new validations can break existing records? Please follow the code quality guidelines about new model validations when adding a new model validation.
If you're adding the validations to a model with no records you can ignore this warning.
8 included do 9 validate :one_issue_tracker, if: :activated?, on: :manual_change 10 11 attribute :category, default: 'issue_tracker' 12 13 before_validation :handle_properties 14 before_validation :set_default_data, on: :create 15 16 # Pattern used to extract links from comments 17 # Override this method on services that uses different patterns 18 # This pattern does not support cross-project references 19 # The other code assumes that this pattern is a superset of all 20 # overridden patterns. See ReferenceRegexes.external_pattern 21 def self.base_reference_pattern(only_long: false) 22 if only_long 23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff
10 11 attribute :category, default: 'issue_tracker' 12 13 before_validation :handle_properties 14 before_validation :set_default_data, on: :create 15 16 # Pattern used to extract links from comments 17 # Override this method on services that uses different patterns 18 # This pattern does not support cross-project references 19 # The other code assumes that this pattern is a superset of all 20 # overridden patterns. See ReferenceRegexes.external_pattern 21 def self.base_reference_pattern(only_long: false) 22 if only_long 23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ 24 else 25 /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff
23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ 24 else 25 /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ 26 end 27 end 28 29 def reference_pattern(only_long: false) 30 self.class.base_reference_pattern(only_long: only_long) 31 end 32 33 def handle_properties 34 # this has been moved from initialize_properties and should be improved 35 # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 36 return unless properties.present? 37 38 safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff
23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ 24 else 25 /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ 26 end 27 end 28 29 def reference_pattern(only_long: false) 30 self.class.base_reference_pattern(only_long: only_long) 31 end 32 33 def handle_properties 34 # this has been moved from initialize_properties and should be improved 35 # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 36 return unless properties.present? 37 38 safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff
23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ 24 else 25 /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ 26 end 27 end 28 29 def reference_pattern(only_long: false) 30 self.class.base_reference_pattern(only_long: only_long) 31 end 32 33 def handle_properties 34 # this has been moved from initialize_properties and should be improved 35 # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 36 return unless properties.present? 37 38 safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff
23 /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ 24 else 25 /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ 26 end 27 end 28 29 def reference_pattern(only_long: false) 30 self.class.base_reference_pattern(only_long: only_long) 31 end 32 33 def handle_properties 34 # this has been moved from initialize_properties and should be improved 35 # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 36 return unless properties.present? 37 38 safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] Found an incorrectly-bounded regex. Ruby regex behavior is multiline by default and lines should be terminated by
This AppSec automation is currently under testing. Use appsec-sasthelpful or appsec-sastunhelpful for quick feedback. For any detailed feedback, add a comment here.\A
for beginning of line and\Z
for end of line, respectively.changed this line in version 4 of the diff