Resolve "Disable GCP free credit banner at instance level"
What does this MR do?
Adds the option to hide third-party offers in the Admin -> Application Settings section.
If this option is left unchecked, i.e., display third party offers, the user will still be able to opt-out of it once they click the "x" on the offer, as it functioned before.
Are there points in the code the reviewer needs to double check?
Nope.
Why was this MR needed?
See issue.
Screenshots (if relevant)
Database Checklist
When adding migrations:
-
Updated db/schema.rb
-
Added a down
method so the migration can be reverted -
Added the output of the migration(s) to the MR body
== 20180704204006 AddHideThirdPartyOffersToApplicationSettings: migrating =====
-- transaction_open?()
-> 0.0000s
-- execute("SET statement_timeout TO 0")
-> 0.0004s
-- transaction()
-- add_column(:application_settings, :hide_third_party_offers, :boolean, {:default=>nil})
-> 0.0026s
-- change_column_default(:application_settings, :hide_third_party_offers, false)
-> 0.0076s
-> 0.0117s
-- transaction_open?()
-> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
-> 0.0007s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
-> 0.0004s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0005s
-- execute("UPDATE \"application_settings\" SET \"hide_third_party_offers\" = 'f' WHERE \"application_settings\".\"id\" >= 1")
-> 0.0012s
-- change_column_null(:application_settings, :hide_third_party_offers, false)
-> 0.0013s
== 20180704204006 AddHideThirdPartyOffersToApplicationSettings: migrated (0.0228s)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added -
Tests added for this feature/bug - Conform by the code review guidelines
-
Has been reviewed by a UX Designer -
Has been reviewed by a Frontend maintainer -
Has been reviewed by a Backend maintainer -
Has been reviewed by a Database specialist
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Conform by the database guides -
If you have multiple commits, please combine them into a few logically organized commits by squashing them -
Internationalization required/considered -
End-to-end tests pass ( package-and-qa
manual pipeline job)
What are the relevant issue numbers?
Closes #48578 (closed)
Merge request reports
Activity
added 1 commit
- e79bd347 - add option to hide third party offers in admin application settings
assigned to @dennis
marked the checklist item Changelog entry added, if necessary as completed
marked the checklist item Conform by the merge request performance guides as completed
marked the checklist item Conform by the style guides as completed
marked the checklist item Conform by the database guides as completed
marked the checklist item If you have multiple commits, please combine them into a few logically organized commits by squashing them as completed
Hi @rymai, would you be available to review this? It should be a quick one!
@tauriedavis this is ready for your review as well!
assigned to @rymai
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
@dennis I left a few notes, LGTM otherwise.
changed milestone to %11.1
assigned to @dennis
Thank you for the notes @rymai! Ready for review again.
assigned to @rymai
added 1 commit
- 36678b41 - do proper check of application settings for `allow_local_requests_from_hooks_and_services`
@dennis Thank you, looks good to me!
@tauriedavis Please review, thanks!
assigned to @tauriedavis
added 63 commits
-
36678b41...2510985f - 61 commits from branch
master
- cf5c5dfb - Merge remote-tracking branch 'origin/master' into…
- 05bfeb21 - Merge branch '48578-disable-gcp-free-credit-banner-at-instance-level' of…
-
36678b41...2510985f - 61 commits from branch
Hi @rymai, do I need to do anything about these failing tests and schema jobs? I'll create the EE port to resolve
ee_compat_check
, but I'm not sure what I should do about the others. Thank you!Ah! Finally, a reason to check in
db/schema.rb
.Will get a database review as well.
Hi @yorickpeterse, would you be able to review the database changes in this MR? Thank you in advance!
assigned to @yorickpeterse
- Resolved by Rémy Coutable
This MR has a corresponding EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6395