Closed kimakan closed 1 year ago
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% | ||
<!-- | Total: | 34 | 40 | 85.0% | --> |
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% | ||
<!-- | Total: | 25 | --> |
Totals | |
---|---|
Change from base Build 5715994248: | 0.2% |
Covered Lines: | 4741 |
Relevant Lines: | 7371 |
@kimakan did you add some docs about this feature in docs repo?
@kimakan did you add some docs about this feature in docs repo?
The documentation will be added after the merge.
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.
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?
I think that is exactly what we need, the makemigration
is obvious and not that of an issue. Good job!
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.)
@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.
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.
@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.
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:In place of
{% show_announcements %}
, the messages will be shown as bootstrap alerts sorted byupdated
time with the most currently updated message on top.The announcement types are
info
,warning
,urgent
which corresponds to the bootstrap alert typesalert-info
,alert-warning
andalert-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.