Skip to content
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

NEW-FEATURE: Announcement messages #200

Merged
merged 5 commits into from
Sep 29, 2023
Merged

Conversation

kimakan
Copy link
Contributor

@kimakan kimakan commented Aug 8, 2023

This feature make it possible to announce maintenance and downtime.

The announcements are defined in contact/model.py. They can be added and updated via the Django admin interface. In order to make the announcements visible on the webpage, the announcement tags must be loaded and used in the templates.
For example:

{% extends 'core/base.html' %}

{% load announcement_tags %}
{% block content %}

<main class="container">
    <section class="wide">
        {% show_announcements %}
        {% block wide %}{% endblock %}
    </section>
</main>

In place of {% show_announcements %}, the messages will be shown as bootstrap alerts sorted by updated time with the most currently updated message on top.

The announcement types are info, warning, urgent which corresponds to the bootstrap alert types alert-info, alert-warning and alert-danger. We may add other types, but I don't think it's necessary since these three types are more than enough for most use cases.

Needed improvements
Currently, the template for the rendered announcement message is barebone and might require some custom styling. It might be even useful to make some of the styling adjustable via scss variables, similar to daiquiri/core/static/core/css/variables.scss. However even in the current state, the template contact/announcements.html can be overloaded by the app anytime and use a custom styling in it.

@kimakan kimakan added this to the Next version milestone Aug 8, 2023
@kimakan kimakan self-assigned this Aug 8, 2023
@kimakan kimakan changed the title add prototype for announcement messages NEW-FEATURE: Announcement messages Aug 8, 2023
@coveralls
Copy link

coveralls commented Aug 8, 2023

Pull Request Test Coverage Report for Build 5795969658

Details

  • 34 of 40 (85.0%) changed or added relevant lines in 4 files are covered.
  • 25 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.2%) to 64.32%

Changes Missing Coverage Covered Lines Changed/Added Lines %
daiquiri/contact/filters.py 4 5 80.0%
daiquiri/contact/models.py 17 18 94.44%
daiquiri/contact/admin.py 8 10 80.0%
daiquiri/contact/templatetags/announcement_tags.py 5 7 71.43%
Files with Coverage Reduction New Missed Lines %
daiquiri/core/renderers/datacite.py 1 3.13%
daiquiri/metadata/serializers/init.py 1 97.44%
daiquiri/contact/filters.py 2 70.0%
daiquiri/contact/models.py 4 86.96%
daiquiri/query/process.py 17 66.67%
Totals Coverage Status
Change from base Build 5715994248: 0.2%
Covered Lines: 4741
Relevant Lines: 7371

💛 - Coveralls

@agy-why
Copy link
Member

agy-why commented Aug 8, 2023

@kimakan did you add some docs about this feature in docs repo?

Copy link
Member

@agy-why agy-why left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with it. Do we have /want an example placeholder for the default app?

@kimakan
Copy link
Contributor Author

kimakan commented Aug 8, 2023

@kimakan did you add some docs about this feature in docs repo?

The documentation will be added after the merge.

@agy-why agy-why modified the milestones: Next version, MiniRelease Aug 25, 2023
@agy-why
Copy link
Member

agy-why commented Sep 13, 2023

add condition possibility on visibility of message, based on preprogrammed filters.

Usecase: show user with consent == False that they need to accept Term of Use.

@kimakan
Copy link
Contributor Author

kimakan commented Sep 19, 2023

add condition possibility on visibility of message, based on preprogrammed filters.

Usecase: show user with consent == False that they need to accept Term of Use.

I came up with a possible solution for this that allows adding new conditions/filters easily.

The filters/conditions are defined in contact/filters.py

   class MessageFilter:

    CHOICES = (
        ("no_filter", "Show to all users"),
        ("user_has_not_consented", "Show to user who has not consented yet"),
    )

    def no_filter(request):
        return True

    def user_has_not_consented(request):
        if request.user.is_authenticated:
            if not request.user.profile.consent:
                return True
        return False                                                    

New conditions can be added directy as methods of the class MessageFilter. Every method must have a single parameter request and the name of the method should be added to CHOICES (otherwise, it will not be added to the model).

The AnnouncementMessage model stores the name of the filter/condition method (it can be selected in Django-Admin)

...
visibility_filter = models.CharField(max_length=30, choices=MessageFilter.CHOICES, default="no_filter")
def get_filter(self):
    return getattr(MessageFilter, str(self.visibility_filter))

...

The actual filtering of the messages happens in the templatetags

@register.inclusion_tag("contact/announcements.html", takes_context=True)
def show_announcements(context):
   request = context["request"]
   announcements = AnnouncementMessage.objects.filter(visible=True)
   announcements = [msg for msg in announcements if msg.get_filter()(request) is True]
   return {
       "announcements": announcements,
   }

This solution requires makemigrations after adding a new filter/condition method in the MessageFilter since new choices are added to the field of the model.
@agy-why What do you think? Is this solution worth pursuing?

@agy-why
Copy link
Member

agy-why commented Sep 19, 2023

I think that is exactly what we need, the makemigration is obvious and not that of an issue. Good job!

@jochenklar
Copy link
Member

Hi @kimakan @agy-why sorry for interupt, running makemigrations on a daiquiri app would create migrations in the daiquiri repo (which is probably only installed by pip in a virtual env). This would break the way daiquiri is set up. (Unless you changed that, I am not up to date anymore, but just saw the email.)

@agy-why
Copy link
Member

agy-why commented Sep 19, 2023

@jochenklar that is a good feed back, thanks! I think we will not add the filtering app wise but globally on daiquiri, but it worth discussing.

@jochenklar
Copy link
Member

If you want choices to be flexible you can implement them like
LICENSE_CHOICES‎: a CharField without choices in the model and choices in the admin.py and in the serializer.

@kimakan
Copy link
Contributor Author

kimakan commented Sep 20, 2023

@jochenklar , Thank you for the feedback! I have implemented the announcement messages following your suggestion.

Now, the new conditions do not require a migration. The default MessageFilter can be overloaded by the app and set by the setting ANNOUNCEMENT_MESSAGE_FILTER.

A new filter can be added as following (i.e. in the filters.py of the app)

from daiquiri.contact.filters import DefaultMessageFilter

class MyAppMessageFilter(DefaultMessageFilter):

    CHOICES = DefaultMessageFilter.CHOICES + (
        ("custom_condition", "Show to the custom group of users"),
    )

    def custom_condition(request):
        return True

And then set the new message filter in the settings with

ANNOUNCEMENT_MESSAGE_FILTER = 'daiquiri.myapp.filters.MyAppMessageFilter'

The current CI tests are failing because the settings from the contact app are not imported by the daiquiri app yet. It will be changed after the merge.

@kimakan kimakan marked this pull request as ready for review September 25, 2023 11:06
@kimakan kimakan merged commit 050719e into master Sep 29, 2023
10 checks passed
@kimakan kimakan deleted the new-feature-announcements branch October 23, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants