Cartodb: Remove duplicate banner in API Keys page

[alejandraarri]

When a user has a notification warning, as the one we are showing with the PG11 migration info, the message shown appears in two different banners in API keys page. The banner below shouldn’t be appearing in this case.

More info can be read in https://github.com/CartoDB/cartodb/pull/14920

[gonzaloriestra]

Hey! We are using the banner for a licensing warning we are going to add in the next on-premises release (this week!), so it would be nice if we could fix it before :angel:

@csubira

[csubira]

What was found:

After a little bit of research I found that the duplicated notification is displayed in the application.html.erb layout which means that since this template is used in many other pages this issue is not only for the API keys page. (In some of the other cases we haven't seen it because it's hidden because of the positioning, or because we haven't checked them)

The pages affected by this issue are the ones rendered by the following admin controllers (All of them from the Settings section in Dashboard):

• client_applications_controller.rb : API keys page
• organization_users_controller.rb and 'organizations_controller.rb': Organizations settings pages
• users_controller.rb: User settings. In _Profile_ and _Account_ pages it is not displayed since the render sets that layout to false and then it is not application the template.
• mobile_apps_controller.rb: Mobile apps pages/Connected Apps

and then from superadmin:

• users_controller.rb
• organizations_controller.rb
• platform_controller.rb
• synchronizations_controller.rb

What is suggested:

My suggestion is to remove the code in application layout where if the user has notifications is checked and the duplicated banner displayed.

✅ PROS:

• The duplication issue is fixed.
• The green notification is still shown in all of the admin pages because it is part of the header imported from the dashboard.
• Other notifications can be shown as Flash Messages through the system that is already implemented.

🚫CONS:

• Are there other kind of messages set in the notifications property of User that we need to take into account?
• What happens with _superadmin_?
  Shall we set an exception to allow this alerts in _superadmin_ (since there is no green notification there)?

@gonzaloriestra @ana-md anything that can help me to strike any of the cons?

[gonzaloriestra]

Thanks for the detailed analysis! :clap:

AFAIK, we only use this kind of notifications with a rake task, adding always a custom message, so I don't see any problem.

Regarding superadmin, I'd say it's ok if we don't show the banner there. It's only for us and we normally use other places to notify about this stuff (like Slack).

@alrocar?

[alrocar]

👍

[ana-md]

Thanks a lot for explaining it so clearly @csubira!!
Agree with @gonzaloriestra about Superadmin, at least from what I have seen on how we use it internally. About the rest, I am not familiar with it at all 😅

[csubira]

Thank you all! Merged and deployed! 🚀