Define banners in the DB

[ewjoachim]

What's the problem this feature will solve?
Setting up and removing occasional community banners seems quite painful

Describe the solution you'd like
There's an ongoing effort to put the Sponsors in the DB (#9512), I'm guessing surfing on the momentum on cleaning the sponsors part, we could also take the opportunity to define those banners in the DB too. I'm even wondering if it would made sense to have the same sponsor roles also able to set banners.

Additional context
I imagine we'd need:

• A model with:
  • A text (potentially with links) that we want to display. XSS may be an interesting problem.
  • A begin and end date, this would allow being more precise on event start and end, and less relying on @ewdurbin 's ability to deploy at specific times
  • Maybe a color if there are choices.
• An admin page to CRUD the banners

There's the question of translations, but as of today, banners are usually up for less time than the interval between weblate PR merges, so I guess most banners are already untranslated as of now.

[ewdurbin]

I'm even wondering if it would made sense to have the same sponsor roles also able to set banners.

Abso-freakin-lutely.

[berinhard]

Hey @ewjoachim, thank for bringing this up! I'd be happy to help with that.

I was thinking on how to implement this and it seems to be simple enough to work on. The how to input links and, thus, potential XSS attacks are definitely something to think about. About the XSS, if the admin expects the text input as a markdown one, we're safe because readme_renderer project takes care of sanitizing it before displaying the final HTML, as I've explained in this comment.

If we agree on the markdown strategy, there's probably one thing we'll have to update on how we're using readme_renderer that is to enable it to consider the class attribute for <a> tags. Currently, it only accepts href and title attributes by default. And, even though the lib's clean method accepts custom allowed attributes, its render one doesn't, and neither warehouse's utils.render. So, probably we'll have to release a new version of the readme_renderer with a more extensible parameters API. Probably the same is true for target="_blank" and rel="noopener" attributes.

This whole readme_renderer thing can also be avoided with @ewjoachim suggestion on having a way to pick colors. But I'll go further and suggest to pick "banners templates" instead of just colors. These templates would be several CSS classes defining the banner color and how a link should be displayed. The classes can be all the notification-bar--* ones. That way, we escape from having to define style within the link content. What do you think?

Also, another question is where this code should live? Is it ok to create a new banners module or is it an overkill since it should probably end up with a simple models.py defining this new model?

[ewdurbin]

I was thinking on how to implement this and it seems to be simple enough to work on. The how to input links and, thus, potential XSS attacks are definitely something to think about. About the XSS, if the admin expects the text input as a markdown one, we're safe because readme_renderer project takes care of sanitizing it before displaying the final HTML, as I've explained in this comment.

We should minimize the featureset here to match the need, at least initially. All historic banners have been a single string describing the banner and a text/link to construct a call-to-action button.

Thus, I don't know that we need much free-form concern around Markdown/rendering.

This whole readme_renderer thing can also be avoided with @ewjoachim suggestion on having a way to pick colors. But I'll go further and suggest to pick "banners templates" instead of just colors. These templates would be several CSS classes defining the banner color and how a link should be displayed. The classes can be all the notification-bar--* ones. That way, we escape from having to define style within the link content. What do you think?

While I think having flexible banners for other uses (important notifications from moderators/admins) is an excellent feature I'm not aware of a use-case for this that we've had in the past that isn't covered by our statuspage. For the initial iteration, reducing the scope would be ideal since we would not want the psf_staff role to be able to apply banners that aren't just the regular yellow.

Also, another question is where this code should live? Is it ok to create a new banners module or is it an overkill since it should probably end up with a simple models.py defining this new model?

banners makes sense to me! i don't think we're adverse to creating new modules.

[ewjoachim]

Ha perfect. I was planning on waiting for your PR to be merged because this one was like going to copy large chunks of it, but you offering to take care of it is perfect. I can promise you a very thorough review in exchange, for what it's worth :) (seriously, that's awesome, thank you)

(Also, +1 Ee on all those points! Start small, iterate)

(❤️ to you all)

[berinhard]

Thanks for all your comments! I'm starting to work on it now and hope to get back with good news as soon as possible 👍

And for @ewjoachim information and to document it here, me and @ewdurbin talked about having a way to preview the banner after it gets created. That way moderators/psf staff/admin users won't have to wait until the banner dates to see the results.

[ewjoachim]

That seems like a nice UX thought