-
Notifications
You must be signed in to change notification settings - Fork 11
Add Newsfeed #507
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
base: main
Are you sure you want to change the base?
Add Newsfeed #507
Changes from all commits
88318cd
1066732
689022d
fcfbbbb
e96a71c
c0d565a
6791863
79ff01b
ac39946
a8f5cf5
14bb3da
e3f6ac2
86b075b
118f63e
02f5d2c
95a97ff
247d3c9
06eabc0
be9e8f8
8abcb98
e808ca8
2cd09d8
8bb5aed
7708394
994d996
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import sys | ||
|
|
||
| import requests | ||
| from django.conf import settings | ||
| from django.contrib.auth.models import Group | ||
| from django.core.management import BaseCommand | ||
|
|
||
| from myhpi.feed.models import NewsFeed, NewsFeedAccount, Post | ||
|
|
||
|
|
||
| def get_release(tag): | ||
| r = requests.get(f"https://api.github.com/repos/fsr-de/myHPI/releases/tags/{tag}") | ||
| r.raise_for_status() | ||
| return r.json() | ||
|
|
||
|
|
||
| def publish_release_post(force_publish, tag=None): | ||
| if tag is None: | ||
| version = settings.MYHPI_VERSION | ||
| tag = f"v{version}" | ||
| release = get_release(tag) | ||
|
|
||
| feed = NewsFeed.objects.first() | ||
| if feed is None: | ||
| print("No feed found, not publishing release post", file=sys.stderr) | ||
| return | ||
|
|
||
| existing_posts = Post.objects.filter(title=release["name"]) | ||
| if existing_posts.exists() and not force_publish: | ||
| print( | ||
| "Release post already exists, not publishing again. Use -f option to create a post anyway.", | ||
| file=sys.stderr, | ||
| ) | ||
| return | ||
|
|
||
| account_id = settings.RELEASE_POST_ACCOUNT_ID | ||
| if account_id is None: | ||
| print("No account id found, not publishing release post", file=sys.stderr) | ||
| return | ||
| account = NewsFeedAccount.objects.filter(id=account_id).first() | ||
| if account is None: | ||
| print("No account found, not publishing release post", file=sys.stderr) | ||
| return | ||
|
|
||
| post = feed.add_child( | ||
| instance=Post( | ||
| title=release["name"], | ||
| body=release["body"], | ||
| post_account=account, | ||
| visible_for=Group.objects.all(), | ||
| is_public=True, | ||
| first_published_at=release["published_at"], | ||
| ) | ||
| ) | ||
| print(f"Published release post with id {post.id}", file=sys.stderr) | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = "Creates a feed post for the latest release of myHPI" | ||
|
|
||
| def add_arguments(self, parser): | ||
| parser.add_argument( | ||
| "--force", | ||
| "-f", | ||
| action="store_true", | ||
| help="Create a release post even if the release is already published.", | ||
| ) | ||
|
|
||
| parser.add_argument( | ||
| "--tag", | ||
| "-t", | ||
| type=str, | ||
| help="Specify a tag to publish a release post for (defaults to currently installed version, typically latest tag).", | ||
| ) | ||
|
|
||
| def handle(self, *args, **options): | ||
| publish_release_post(options["force"], options["tag"]) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| from django.http import HttpResponseRedirect | ||
|
|
||
| from myhpi import settings | ||
| from myhpi.feed.models import NewsFeed | ||
|
|
||
|
|
||
| class FeedRedirectMiddleware: | ||
| def __init__(self, get_response): | ||
| self.get_response = get_response | ||
| self.feed = NewsFeed.objects.first() | ||
|
|
||
| def __call__(self, request): | ||
| self.process_request(request) | ||
| return self.get_response(request) | ||
|
|
||
| def process_request(self, request): | ||
| user = request.user | ||
| if ( | ||
| self.feed | ||
| and request.path == "/" | ||
| and user.is_authenticated | ||
| and self.feed.visible_for.intersection(user.groups.all()).exists() | ||
| and request.GET.get("no_redirect") != "true" | ||
| and settings.REDIRECT_TO_FEED | ||
| ): | ||
| request.path = self.feed.slug | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| # Generated by Django 4.2.9 on 2024-02-15 17:16 | ||
|
|
||
| import django.db.models.deletion | ||
| import wagtail.users.models | ||
| from django.db import migrations, models | ||
|
|
||
| import myhpi.core.markdown.fields | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| initial = True | ||
|
|
||
| dependencies = [ | ||
| ("core", "0010_minutes_location"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="NewsFeed", | ||
| fields=[ | ||
| ( | ||
| "basepage_ptr", | ||
| models.OneToOneField( | ||
| auto_created=True, | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| parent_link=True, | ||
| primary_key=True, | ||
| serialize=False, | ||
| to="core.basepage", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "abstract": False, | ||
| }, | ||
| bases=("core.basepage",), | ||
| ), | ||
| migrations.CreateModel( | ||
| name="NewsFeedAccount", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| models.AutoField( | ||
| auto_created=True, primary_key=True, serialize=False, verbose_name="ID" | ||
| ), | ||
| ), | ||
| ("post_key", models.CharField(max_length=255)), | ||
| ("name", models.CharField(max_length=255)), | ||
| ( | ||
| "icon", | ||
| models.ImageField( | ||
| blank=True, null=True, upload_to=wagtail.users.models.upload_avatar_to | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| migrations.CreateModel( | ||
| name="Post", | ||
| fields=[ | ||
| ( | ||
| "basepage_ptr", | ||
| models.OneToOneField( | ||
| auto_created=True, | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| parent_link=True, | ||
| primary_key=True, | ||
| serialize=False, | ||
| to="core.basepage", | ||
| ), | ||
| ), | ||
| ("body", myhpi.core.markdown.fields.CustomMarkdownField()), | ||
| ( | ||
| "post_account", | ||
| models.ForeignKey( | ||
| blank=True, | ||
| null=True, | ||
| on_delete=django.db.models.deletion.SET_NULL, | ||
| related_name="posts", | ||
| to="feed.newsfeedaccount", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "abstract": False, | ||
| }, | ||
| bases=("core.basepage",), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| from django.core.paginator import Paginator | ||
| from django.db import models | ||
| from wagtail.admin.panels import FieldPanel | ||
| from wagtail.snippets.models import register_snippet | ||
| from wagtail.users.models import UserProfile, upload_avatar_to | ||
|
|
||
| from myhpi import settings | ||
| from myhpi.core.markdown.fields import CustomMarkdownField | ||
| from myhpi.core.models import BasePage | ||
|
|
||
|
|
||
| class NewsFeed(BasePage): | ||
| parent_page_types = ["core.RootPage"] | ||
| subpage_types = ["Post"] | ||
| max_count = 1 | ||
|
|
||
| def get_context(self, request, *args, **kwargs): | ||
| context = super().get_context(request, *args, **kwargs) | ||
| all_posts = self.get_children().live().order_by("-first_published_at").specific() | ||
| paginator = Paginator(all_posts, 10) | ||
| page = paginator.get_page(request.GET.get("page")) | ||
|
|
||
| # Currently, there is no filter for visibility on posts. That is, it is assumed that all post should be visible to students | ||
| context["page"] = page | ||
| context["posts"] = page.object_list | ||
| context["limit"] = settings.FEED_TRUNCATE_LIMIT | ||
| return context | ||
|
|
||
|
|
||
| class Post(BasePage): | ||
| parent_page_types = ["NewsFeed"] | ||
| subpage_types = [] | ||
| body = CustomMarkdownField() | ||
| post_account = models.ForeignKey( | ||
| "NewsFeedAccount", related_name="posts", on_delete=models.SET_NULL, null=True, blank=True | ||
| ) | ||
|
|
||
| content_panels = BasePage.content_panels + [ | ||
| FieldPanel("body", classname="full"), | ||
| ] | ||
|
|
||
| @property | ||
| def author(self): | ||
| if self.last_edited_by: | ||
| return self.last_edited_by | ||
| else: | ||
| return self.post_account | ||
|
|
||
| @property | ||
| def date(self): | ||
| if self.first_published_at: | ||
| return self.first_published_at.date() | ||
|
|
||
| @property | ||
| def image_url(self): | ||
| if self.post_account and self.post_account.icon: | ||
| return self.post_account.icon.url | ||
| # Currently does not use the wagtail user icon since it is not used anywhere else | ||
| else: | ||
| return "https://www.gravatar.com/avatar/b1a83a1baa8a1d45c905a59217a7d30a?s=140&d=mm" | ||
|
|
||
|
|
||
| # The NewsFeedAccount is used to create posts via API | ||
| @register_snippet | ||
| class NewsFeedAccount(models.Model): | ||
| post_key = models.CharField(max_length=255) | ||
| name = models.CharField(max_length=255) | ||
|
|
||
| icon = models.ImageField(upload_to=upload_avatar_to, null=True, blank=True) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this the correct reference to also use all of the wagtail stuff for images?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same code that is used for uploading user avatars in Wagtail. Does not have to deal with collections etc., which makes sense because the post accounts should only be edited by admins anyway. |
||
|
|
||
| def __str__(self): | ||
| return self.name | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could also implement this inside the serve() of RootPage, that would only do the check when it is actually necessary and not with every request
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that would increase coupling. Currently disabling/removing the app is very easy, it is never mentioned / used in core. Your thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, it would be okay to introduce some change to core, as myHPI (this application) is developed specifically for our myHPI website and not meant as a generic CMS. Hence, we don't need to be able to deactivate/remove the newsfeed app per installation and that small amount of coupling would be acceptable.
However, I'm not that familiar with Django patterns, so I can't really argue on whether there is even a need to replace this middleware or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukasrad02 Felix and I had a conversation about the conflict of cool new features vs. size of codebase/maintainability. I made the argument that as long as fancy features are loosely coupled, future maintainer could easily remove them without the need to understand/update/fix them and therefore delay or block the maintenance of the core app.
The middleware is probably fine, as it doesn't do much. I just thought about it because we always struggle with pageload times and this is code that is not necessary on all pages but the root page.
Another way that we could pursue this is by adding some kind of "plugin" mechanism. Again citing ephios: we are sending out custom django signals to "plugins" (other django apps within the same repo or installed separately) and let them inject stuff at various places. e.g. there is a signal that says "hey plugin, give me some html to append to the homepage if you want to". This way, the additional code is only run when it is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with you two deciding on the best approach to do this. Especially when it comes to performance or plugins in Django, I would not be able to give any qualified arguments.