From 36a461b57c5b901fb5cfb81966bac3f0387fd590 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 11 Jul 2022 05:05:51 +0400 Subject: Bump flake8-bandit To v3 Bumps flake-bandit to v3 to fix an incompatibility with the bandit package. This also bumps flake8-annotations to a legally acceptable version, which introduces ANN401, which disallows `typing.Any` annotations (for the most part, refer to the docs). Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/views/page_category.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/views/page_category.py b/pydis_site/apps/content/views/page_category.py index 5af77aff..356eb021 100644 --- a/pydis_site/apps/content/views/page_category.py +++ b/pydis_site/apps/content/views/page_category.py @@ -3,7 +3,7 @@ from pathlib import Path import frontmatter from django.conf import settings -from django.http import Http404 +from django.http import Http404, HttpRequest, HttpResponse from django.views.generic import TemplateView from pydis_site.apps.content import utils @@ -12,7 +12,7 @@ from pydis_site.apps.content import utils class PageOrCategoryView(TemplateView): """Handles pages and page categories.""" - def dispatch(self, request: t.Any, *args, **kwargs) -> t.Any: + def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: """Conform URL path location to the filesystem path.""" self.location = Path(kwargs.get("location", "")) -- cgit v1.2.3 From 4edb368c39c96f20e41a8874b0f77f30910c8ce7 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 11 Jul 2022 05:10:19 +0400 Subject: Bump Django To 3.2 Bumps the current django version to 3.2 For the most part, there are no major changes here. The app configuration names were incorrect, and the new version no longer ignores that, so those were updated. The new version also requires explicitly defining the field type for primary keys if they are not defined on the model. Signed-off-by: Hassan Abouelela --- poetry.lock | 13 +++++-------- pydis_site/apps/content/apps.py | 2 +- pydis_site/apps/events/apps.py | 2 +- pydis_site/apps/redirect/apps.py | 2 +- pydis_site/apps/resources/apps.py | 2 +- pydis_site/apps/staff/apps.py | 2 +- pydis_site/settings.py | 3 +++ pyproject.toml | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/poetry.lock b/poetry.lock index 826c4fca..d855bdaf 100644 --- a/poetry.lock +++ b/poetry.lock @@ -131,19 +131,19 @@ python-versions = "*" [[package]] name = "django" -version = "3.1.14" +version = "3.2.14" description = "A high-level Python Web framework that encourages rapid development and clean, pragmatic design." category = "main" optional = false python-versions = ">=3.6" [package.dependencies] -asgiref = ">=3.2.10,<4" +asgiref = ">=3.3.2,<4" pytz = "*" sqlparse = ">=0.2.2" [package.extras] -argon2 = ["argon2-cffi (>=16.1.0)"] +argon2 = ["argon2-cffi (>=19.1.0)"] bcrypt = ["bcrypt"] [[package]] @@ -867,7 +867,7 @@ testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest- [metadata] lock-version = "1.1" python-versions = "3.9.*" -content-hash = "91913e2e96ab2e0e78a09334241062359605135d64f458e710f66d00fb670e05" +content-hash = "e40d87f94732c314e0fb3cd2c9023ea47d66d3c3d340085eb6df9c1fe1412529" [metadata.files] anyio = [ @@ -958,10 +958,7 @@ distlib = [ {file = "distlib-0.3.4-py2.py3-none-any.whl", hash = "sha256:6564fe0a8f51e734df6333d08b8b94d4ea8ee6b99b5ed50613f731fd4089f34b"}, {file = "distlib-0.3.4.zip", hash = "sha256:e4b58818180336dc9c529bfb9a0b58728ffc09ad92027a3f30b7cd91e3458579"}, ] -django = [ - {file = "Django-3.1.14-py3-none-any.whl", hash = "sha256:0fabc786489af16ad87a8c170ba9d42bfd23f7b699bd5ef05675864e8d012859"}, - {file = "Django-3.1.14.tar.gz", hash = "sha256:72a4a5a136a214c39cf016ccdd6b69e2aa08c7479c66d93f3a9b5e4bb9d8a347"}, -] +django = [] django-distill = [ {file = "django-distill-2.9.2.tar.gz", hash = "sha256:91d5f45c2ff78b8efd4805ff5ec17df4ba815bbf51ca12a2cea65727d2f1d42e"}, ] diff --git a/pydis_site/apps/content/apps.py b/pydis_site/apps/content/apps.py index 1e300a48..96019e1c 100644 --- a/pydis_site/apps/content/apps.py +++ b/pydis_site/apps/content/apps.py @@ -4,4 +4,4 @@ from django.apps import AppConfig class ContentConfig(AppConfig): """Django AppConfig for content app.""" - name = 'content' + name = 'pydis_site.apps.content' diff --git a/pydis_site/apps/events/apps.py b/pydis_site/apps/events/apps.py index a1cf09ef..70762bc2 100644 --- a/pydis_site/apps/events/apps.py +++ b/pydis_site/apps/events/apps.py @@ -4,4 +4,4 @@ from django.apps import AppConfig class EventsConfig(AppConfig): """Django AppConfig for events app.""" - name = 'events' + name = 'pydis_site.apps.events' diff --git a/pydis_site/apps/redirect/apps.py b/pydis_site/apps/redirect/apps.py index 9b70d169..0234bc93 100644 --- a/pydis_site/apps/redirect/apps.py +++ b/pydis_site/apps/redirect/apps.py @@ -4,4 +4,4 @@ from django.apps import AppConfig class RedirectConfig(AppConfig): """AppConfig instance for Redirect app.""" - name = 'redirect' + name = 'pydis_site.apps.redirect' diff --git a/pydis_site/apps/resources/apps.py b/pydis_site/apps/resources/apps.py index e0c235bd..93117654 100644 --- a/pydis_site/apps/resources/apps.py +++ b/pydis_site/apps/resources/apps.py @@ -4,4 +4,4 @@ from django.apps import AppConfig class ResourcesConfig(AppConfig): """AppConfig instance for Resources app.""" - name = 'resources' + name = 'pydis_site.apps.resources' diff --git a/pydis_site/apps/staff/apps.py b/pydis_site/apps/staff/apps.py index 70a15f40..d68a80c3 100644 --- a/pydis_site/apps/staff/apps.py +++ b/pydis_site/apps/staff/apps.py @@ -4,4 +4,4 @@ from django.apps import AppConfig class StaffConfig(AppConfig): """Django AppConfig for the staff app.""" - name = 'staff' + name = 'pydis_site.apps.staff' diff --git a/pydis_site/settings.py b/pydis_site/settings.py index 17f220f3..03c16f4b 100644 --- a/pydis_site/settings.py +++ b/pydis_site/settings.py @@ -219,6 +219,9 @@ if DEBUG: else: PARENT_HOST = env('PARENT_HOST', default='pythondiscord.com') +# Django Model Configuration +DEFAULT_AUTO_FIELD = "django.db.models.AutoField" + # Django REST framework # https://www.django-rest-framework.org REST_FRAMEWORK = { diff --git a/pyproject.toml b/pyproject.toml index 6ef7c407..75292eb2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ license = "MIT" [tool.poetry.dependencies] python = "3.9.*" -django = "~=3.1.14" +django = "~=3.2.14" django-environ = "~=0.4.5" django-filter = "~=21.1" djangorestframework = "~=3.12.0" -- cgit v1.2.3 From d50028d6b92909a39139007f0f3bcd7c90a88420 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 06:08:22 +0200 Subject: Add Tags To Content Listings Adds bot tags to the content page, as well as a model to go along with it. Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/migrations/0001_initial.py | 23 ++++ pydis_site/apps/content/migrations/__init__.py | 0 pydis_site/apps/content/models/__init__.py | 0 pydis_site/apps/content/models/tag.py | 17 +++ pydis_site/apps/content/resources/tags/_info.yml | 3 + pydis_site/apps/content/utils.py | 125 ++++++++++++++++++++- pydis_site/apps/content/views/page_category.py | 9 +- pydis_site/templates/content/listing.html | 10 +- pydis_site/templates/content/page.html | 2 +- 9 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 pydis_site/apps/content/migrations/0001_initial.py create mode 100644 pydis_site/apps/content/migrations/__init__.py create mode 100644 pydis_site/apps/content/models/__init__.py create mode 100644 pydis_site/apps/content/models/tag.py create mode 100644 pydis_site/apps/content/resources/tags/_info.yml (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/migrations/0001_initial.py b/pydis_site/apps/content/migrations/0001_initial.py new file mode 100644 index 00000000..15e3fc95 --- /dev/null +++ b/pydis_site/apps/content/migrations/0001_initial.py @@ -0,0 +1,23 @@ +# Generated by Django 4.0.6 on 2022-08-13 00:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='Tag', + fields=[ + ('last_updated', models.DateTimeField(auto_now=True, help_text='The date and time this data was last fetched.')), + ('name', models.CharField(help_text="The tag's name.", max_length=50, primary_key=True, serialize=False)), + ('body', models.TextField(help_text='The content of the tag.')), + ('url', models.URLField(help_text='The URL to this tag on GitHub.')), + ], + ), + ] diff --git a/pydis_site/apps/content/migrations/__init__.py b/pydis_site/apps/content/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/pydis_site/apps/content/models/__init__.py b/pydis_site/apps/content/models/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py new file mode 100644 index 00000000..1437b96a --- /dev/null +++ b/pydis_site/apps/content/models/tag.py @@ -0,0 +1,17 @@ +from django.db import models + + +class Tag(models.Model): + """A tag from the python-discord server.""" + + last_updated = models.DateTimeField( + help_text="The date and time this data was last fetched.", + auto_now=True, + ) + name = models.CharField( + help_text="The tag's name.", + primary_key=True, + max_length=50, + ) + body = models.TextField(help_text="The content of the tag.") + url = models.URLField(help_text="The URL to this tag on GitHub.") diff --git a/pydis_site/apps/content/resources/tags/_info.yml b/pydis_site/apps/content/resources/tags/_info.yml new file mode 100644 index 00000000..054125ec --- /dev/null +++ b/pydis_site/apps/content/resources/tags/_info.yml @@ -0,0 +1,3 @@ +title: Tags +description: Useful snippets that are often used in the server. +icon: fas fa-tags diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index d3f270ff..a4252284 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -1,14 +1,26 @@ +import datetime +import functools +import tarfile +import tempfile +from io import BytesIO from pathlib import Path -from typing import Dict, Tuple import frontmatter +import httpx import markdown import yaml from django.http import Http404 +from django.utils import timezone from markdown.extensions.toc import TocExtension +from pydis_site import settings +from .models.tag import Tag -def get_category(path: Path) -> Dict[str, str]: +TAG_URL_BASE = "https://github.com/python-discord/bot/tree/main/bot/resources/tags" +TAG_CACHE_TTL = datetime.timedelta(hours=1) + + +def get_category(path: Path) -> dict[str, str]: """Load category information by name from _info.yml.""" if not path.is_dir(): raise Http404("Category not found.") @@ -16,7 +28,7 @@ def get_category(path: Path) -> Dict[str, str]: return yaml.safe_load(path.joinpath("_info.yml").read_text(encoding="utf-8")) -def get_categories(path: Path) -> Dict[str, Dict]: +def get_categories(path: Path) -> dict[str, dict]: """Get information for all categories.""" categories = {} @@ -27,8 +39,111 @@ def get_categories(path: Path) -> Dict[str, Dict]: return categories -def get_category_pages(path: Path) -> Dict[str, Dict]: +@functools.cache +def get_tags_static() -> list[Tag]: + """ + Fetch tag information in static builds. + + This will return a cached value, so it should only be used for static builds. + """ + return fetch_tags() + + +def fetch_tags() -> list[Tag]: + """ + Fetch tag data from the GitHub API. + + The entire repository is downloaded and extracted locally because + getting file content would require one request per file, and can get rate-limited. + """ + if settings.GITHUB_TOKEN: + headers = {"Authorization": f"token {settings.GITHUB_TOKEN}"} + else: + headers = {} + + tar_file = httpx.get( + f"{settings.GITHUB_API}/repos/python-discord/bot/tarball", + follow_redirects=True, + timeout=settings.TIMEOUT_PERIOD, + headers=headers, + ) + tar_file.raise_for_status() + + tags = [] + with tempfile.TemporaryDirectory() as folder: + with tarfile.open(fileobj=BytesIO(tar_file.content)) as repo: + included = [] + for file in repo.getmembers(): + if "/bot/resources/tags" in file.path: + included.append(file) + repo.extractall(folder, included) + + for tag_file in Path(folder).rglob("*.md"): + tags.append(Tag( + name=tag_file.name.removesuffix(".md"), + body=tag_file.read_text(encoding="utf-8"), + url=f"{TAG_URL_BASE}/{tag_file.name}" + )) + + return tags + + +def get_tags() -> list[Tag]: + """Return a list of all tags visible to the application, from the cache or API.""" + if settings.STATIC_BUILD: + last_update = None + else: + last_update = ( + Tag.objects.values_list("last_updated", flat=True) + .order_by("last_updated").first() + ) + + if last_update is None or timezone.now() >= (last_update + TAG_CACHE_TTL): + # Stale or empty cache + if settings.STATIC_BUILD: + tags = get_tags_static() + else: + tags = fetch_tags() + Tag.objects.exclude(name__in=[tag.name for tag in tags]).delete() + for tag in tags: + tag.save() + + return tags + else: + # Get tags from database + return Tag.objects.all() + + +def get_tag(name: str) -> Tag: + """Return a tag by name.""" + tags = get_tags() + for tag in tags: + if tag.name == name: + return tag + + raise Tag.DoesNotExist() + + +def get_category_pages(path: Path) -> dict[str, dict]: """Get all page names and their metadata at a category path.""" + # Special handling for tags + if path == Path(__file__).parent / "resources/tags": + tags = {} + for tag in get_tags(): + content = frontmatter.parse(tag.body)[1] + if len(content) > 100: + # Trim the preview to a maximum of 100 visible characters + # This causes some markdown to break, but we ignore that + content = content[:100] + "..." + + tags[tag.name] = { + "title": tag.name, + "description": markdown.markdown(content), + "icon": "fas fa-tag" + } + + return {name: tags[name] for name in sorted(tags)} + pages = {} for item in path.glob("*.md"): @@ -39,7 +154,7 @@ def get_category_pages(path: Path) -> Dict[str, Dict]: return pages -def get_page(path: Path) -> Tuple[str, Dict]: +def get_page(path: Path) -> tuple[str, dict]: """Get one specific page.""" if not path.is_file(): raise Http404("Page not found.") diff --git a/pydis_site/apps/content/views/page_category.py b/pydis_site/apps/content/views/page_category.py index 356eb021..01ce8402 100644 --- a/pydis_site/apps/content/views/page_category.py +++ b/pydis_site/apps/content/views/page_category.py @@ -1,4 +1,3 @@ -import typing as t from pathlib import Path import frontmatter @@ -25,7 +24,7 @@ class PageOrCategoryView(TemplateView): return super().dispatch(request, *args, **kwargs) - def get_template_names(self) -> t.List[str]: + def get_template_names(self) -> list[str]: """Checks if the view uses the page template or listing template.""" if self.page_path.is_file(): template_name = "content/page.html" @@ -36,7 +35,7 @@ class PageOrCategoryView(TemplateView): return [template_name] - def get_context_data(self, **kwargs) -> t.Dict[str, t.Any]: + def get_context_data(self, **kwargs) -> dict[str, any]: """Assign proper context variables based on what resource user requests.""" context = super().get_context_data(**kwargs) @@ -73,7 +72,7 @@ class PageOrCategoryView(TemplateView): return context @staticmethod - def _get_page_context(path: Path) -> t.Dict[str, t.Any]: + def _get_page_context(path: Path) -> dict[str, any]: page, metadata = utils.get_page(path) return { "page": page, @@ -84,7 +83,7 @@ class PageOrCategoryView(TemplateView): } @staticmethod - def _get_category_context(path: Path) -> t.Dict[str, t.Any]: + def _get_category_context(path: Path) -> dict[str, any]: category = utils.get_category(path) return { "categories": utils.get_categories(path), diff --git a/pydis_site/templates/content/listing.html b/pydis_site/templates/content/listing.html index ef0ef919..eeb6b5e2 100644 --- a/pydis_site/templates/content/listing.html +++ b/pydis_site/templates/content/listing.html @@ -1,6 +1,8 @@ +{# Base navigation screen for resources #} {% extends 'content/base.html' %} {% block page_content %} + {# Nested Categories #} {% for category, data in categories.items %}
@@ -13,6 +15,8 @@

{{ data.description }}

{% endfor %} + + {# Single Pages #} {% for page, data in pages.items %}
@@ -21,7 +25,11 @@ {{ data.title }} -

{{ data.description }}

+ {% if "tags" in location %} +

{{ data.description | safe }}

+ {% else %} +

{{ data.description }}

+ {% endif %}
{% endfor %} {% endblock %} diff --git a/pydis_site/templates/content/page.html b/pydis_site/templates/content/page.html index 759286f6..625c01f1 100644 --- a/pydis_site/templates/content/page.html +++ b/pydis_site/templates/content/page.html @@ -5,7 +5,7 @@ - + {% endblock %} {% block page_content %} -- cgit v1.2.3 From 42124deb7ea5f17bc6faf959baba8e951b567655 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 06:10:38 +0200 Subject: Add Tag Page Template Add a template for the tag page itself, and add a route to use it. Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/urls.py | 21 ++++++++++-- pydis_site/apps/content/views/__init__.py | 3 +- pydis_site/apps/content/views/tags.py | 56 +++++++++++++++++++++++++++++++ pydis_site/static/css/content/tag.css | 7 ++++ pydis_site/templates/content/base.html | 2 +- pydis_site/templates/content/tag.html | 21 ++++++++++++ 6 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 pydis_site/apps/content/views/tags.py create mode 100644 pydis_site/static/css/content/tag.css create mode 100644 pydis_site/templates/content/tag.html (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/urls.py b/pydis_site/apps/content/urls.py index f8496095..b4ffc07d 100644 --- a/pydis_site/apps/content/urls.py +++ b/pydis_site/apps/content/urls.py @@ -3,7 +3,7 @@ from pathlib import Path from django_distill import distill_path -from . import views +from . import utils, views app_name = "content" @@ -29,18 +29,33 @@ def __get_all_files(root: Path, folder: typing.Optional[Path] = None) -> list[st return results -def get_all_pages() -> typing.Iterator[dict[str, str]]: +DISTILL_RETURN = typing.Iterator[dict[str, str]] + + +def get_all_pages() -> DISTILL_RETURN: """Yield a dict of all page categories.""" for location in __get_all_files(Path("pydis_site", "apps", "content", "resources")): yield {"location": location} +def get_all_tags() -> DISTILL_RETURN: + """Return all tag names in the repository in static builds.""" + for tag in utils.get_tags_static(): + yield {"name": tag.name} + + urlpatterns = [ distill_path("", views.PageOrCategoryView.as_view(), name='pages'), + distill_path( + "tags//", + views.TagView.as_view(), + name="tag", + distill_func=get_all_tags + ), distill_path( "/", views.PageOrCategoryView.as_view(), name='page_category', distill_func=get_all_pages - ), + ) ] diff --git a/pydis_site/apps/content/views/__init__.py b/pydis_site/apps/content/views/__init__.py index 70ea1c7a..a969b1dc 100644 --- a/pydis_site/apps/content/views/__init__.py +++ b/pydis_site/apps/content/views/__init__.py @@ -1,3 +1,4 @@ from .page_category import PageOrCategoryView +from .tags import TagView -__all__ = ["PageOrCategoryView"] +__all__ = ["PageOrCategoryView", "TagView"] diff --git a/pydis_site/apps/content/views/tags.py b/pydis_site/apps/content/views/tags.py new file mode 100644 index 00000000..12e311dc --- /dev/null +++ b/pydis_site/apps/content/views/tags.py @@ -0,0 +1,56 @@ +import re + +import frontmatter +import markdown +from django.conf import settings +from django.http import Http404 +from django.urls import reverse +from django.views.generic import TemplateView + +from pydis_site.apps.content import utils +from pydis_site.apps.content.models.tag import Tag + +COMMAND_REGEX = re.compile(r"`*!tags? (?P[\w\d-]+)`*") + + +class TagView(TemplateView): + """Handles tag pages.""" + + template_name = "content/tag.html" + + def get_context_data(self, **kwargs) -> dict: + """Get the relevant context for this tag page.""" + try: + tag = utils.get_tag(kwargs.get("name")) + except Tag.DoesNotExist: + raise Http404 + + context = super().get_context_data(**kwargs) + context["page_title"] = tag.name + body = frontmatter.parse(tag.body) + content = body[1] + + # Check for tags which can be hyperlinked + start = 0 + while match := COMMAND_REGEX.search(content, start): + link = reverse("content:tag", kwargs={"name": match.group("name")}) + content = content[:match.start()] + f"[{match.group()}]({link})" + content[match.end():] + start = match.end() + + # Add support for some embed elements + if embed := body[0].get("embed"): + context["page_title"] = embed["title"] + if image := embed.get("image"): + content = f"![{embed['title']}]({image['url']})\n\n" + content + + context.update({ + "page": markdown.markdown(content, extensions=["pymdownx.superfences"]), + "tag": tag, + }) + + context["breadcrumb_items"] = [{ + "name": utils.get_category(settings.CONTENT_PAGES_PATH / location)["title"], + "path": str(location) + } for location in [".", "tags"]] + + return context diff --git a/pydis_site/static/css/content/tag.css b/pydis_site/static/css/content/tag.css new file mode 100644 index 00000000..a144ce24 --- /dev/null +++ b/pydis_site/static/css/content/tag.css @@ -0,0 +1,7 @@ +h1.title a { + color: black; +} + +h1.title a:hover { + color: #7289DA; +} diff --git a/pydis_site/templates/content/base.html b/pydis_site/templates/content/base.html index 4a19a275..dbd303a1 100644 --- a/pydis_site/templates/content/base.html +++ b/pydis_site/templates/content/base.html @@ -35,7 +35,7 @@
-

{{ page_title }}

+

{% block title_element %}{{ page_title }}{% endblock %}

{% block page_content %}{% endblock %}
diff --git a/pydis_site/templates/content/tag.html b/pydis_site/templates/content/tag.html new file mode 100644 index 00000000..264f63d0 --- /dev/null +++ b/pydis_site/templates/content/tag.html @@ -0,0 +1,21 @@ +{% extends "content/page.html" %} +{% load static %} + +{% block head %} + {{ block.super }} + + {{ tag.name }} +{% endblock %} + +{% block title_element %} +
+
{{ block.super }}
+
+ +
+
+{% endblock %} + +{% block page_content %} + {{ block.super }} +{% endblock %} -- cgit v1.2.3 From 733d5c084b1cba91a76f495b37ef0a391e5f9900 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 06:43:05 +0200 Subject: Export Tag Model As Top Level Model Object Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/models/__init__.py | 3 +++ pydis_site/apps/content/utils.py | 2 +- pydis_site/apps/content/views/tags.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/models/__init__.py b/pydis_site/apps/content/models/__init__.py index e69de29b..2718ce94 100644 --- a/pydis_site/apps/content/models/__init__.py +++ b/pydis_site/apps/content/models/__init__.py @@ -0,0 +1,3 @@ +from .tag import Tag + +__all__ = ["Tag"] diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index a4252284..de609596 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -14,7 +14,7 @@ from django.utils import timezone from markdown.extensions.toc import TocExtension from pydis_site import settings -from .models.tag import Tag +from .models import Tag TAG_URL_BASE = "https://github.com/python-discord/bot/tree/main/bot/resources/tags" TAG_CACHE_TTL = datetime.timedelta(hours=1) diff --git a/pydis_site/apps/content/views/tags.py b/pydis_site/apps/content/views/tags.py index 12e311dc..e2cfb488 100644 --- a/pydis_site/apps/content/views/tags.py +++ b/pydis_site/apps/content/views/tags.py @@ -8,7 +8,7 @@ from django.urls import reverse from django.views.generic import TemplateView from pydis_site.apps.content import utils -from pydis_site.apps.content.models.tag import Tag +from pydis_site.apps.content.models import Tag COMMAND_REGEX = re.compile(r"`*!tags? (?P[\w\d-]+)`*") -- cgit v1.2.3 From a8fd8b823748ced96bb8ea34e8dcd8bd0dd57671 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 08:47:29 +0200 Subject: Add Tag View Tests Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/tests/test_utils.py | 103 +++++++++++++++++++++- pydis_site/apps/content/tests/test_views.py | 128 ++++++++++++++++++++++++++++ pydis_site/apps/content/utils.py | 6 +- 3 files changed, 233 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index be5ea897..89ef81c4 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -1,8 +1,16 @@ +import tarfile +import tempfile +import textwrap from pathlib import Path +from unittest import mock +import httpx +import markdown from django.http import Http404 +from django.test import TestCase -from pydis_site.apps.content import utils +from pydis_site import settings +from pydis_site.apps.content import models, utils from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) @@ -96,3 +104,96 @@ class GetPageTests(MockPagesTestCase): def test_get_nonexistent_page_returns_404(self): with self.assertRaises(Http404): utils.get_page(Path(BASE_PATH, "invalid")) + + +class TagUtilsTests(TestCase): + """Tests for the tag-related utilities.""" + + @mock.patch.object(utils, "fetch_tags") + def test_static_fetch(self, fetch_mock: mock.Mock): + """Test that the static fetch function is only called at most once during static builds.""" + tags = [models.Tag(name="Name", body="body", url="url")] + fetch_mock.return_value = tags + result = utils.get_tags_static() + second_result = utils.get_tags_static() + + fetch_mock.assert_called_once() + self.assertEqual(tags, result) + self.assertEqual(tags, second_result) + + @mock.patch("httpx.get") + def test_mocked_fetch(self, get_mock: mock.Mock): + """Test that proper data is returned from fetch, but with a mocked API response.""" + bodies = ( + "This is the first tag!", + textwrap.dedent(""" + --- + frontmatter: empty + --- + This tag has frontmatter! + """), + ) + + # Generate a tar archive with a few tags + with tempfile.TemporaryDirectory() as tar_folder: + tar_folder = Path(tar_folder) + with tempfile.TemporaryDirectory() as folder: + folder = Path(folder) + (folder / "ignored_file.md").write_text("This is an ignored file.") + tags_folder = folder / "bot/resources/tags" + tags_folder.mkdir(parents=True) + + (tags_folder / "first_tag.md").write_text(bodies[0]) + (tags_folder / "second_tag.md").write_text(bodies[1]) + + with tarfile.open(tar_folder / "temp.tar", "w") as file: + file.add(folder, recursive=True) + + body = (tar_folder / "temp.tar").read_bytes() + + get_mock.return_value = httpx.Response( + status_code=200, + content=body, + request=httpx.Request("GET", "https://google.com"), + ) + + result = utils.fetch_tags() + self.assertEqual([ + models.Tag(name="first_tag", body=bodies[0], url=f"{utils.TAG_URL_BASE}/first_tag.md"), + models.Tag(name="second_tag", body=bodies[1], url=f"{utils.TAG_URL_BASE}/first_tag.md"), + ], sorted(result, key=lambda tag: tag.name)) + + def test_get_real_tag(self): + """Test that a single tag is returned if it exists.""" + tag = models.Tag.objects.create(name="real-tag") + result = utils.get_tag("real-tag") + + self.assertEqual(tag, result) + + def test_get_tag_404(self): + """Test that an error is raised when we fetch a non-existing tag.""" + models.Tag.objects.create(name="real-tag") + with self.assertRaises(models.Tag.DoesNotExist): + utils.get_tag("fake") + + def test_category_pages(self): + """Test that the category pages function returns the correct records for tags.""" + models.Tag.objects.create(name="second-tag", body="Normal body") + models.Tag.objects.create(name="first-tag", body="Normal body") + tag_body = {"description": markdown.markdown("Normal body"), "icon": "fas fa-tag"} + + result = utils.get_category_pages(settings.CONTENT_PAGES_PATH / "tags") + self.assertDictEqual({ + "first-tag": {**tag_body, "title": "first-tag"}, + "second-tag": {**tag_body, "title": "second-tag"}, + }, result) + + def test_trimmed_tag_content(self): + """Test a tag with a long body that requires trimming.""" + tag = models.Tag.objects.create(name="long-tag", body="E" * 300) + result = utils.get_category_pages(settings.CONTENT_PAGES_PATH / "tags") + self.assertDictEqual({"long-tag": { + "title": "long-tag", + "description": markdown.markdown(tag.body[:100] + "..."), + "icon": "fas fa-tag", + }}, result) diff --git a/pydis_site/apps/content/tests/test_views.py b/pydis_site/apps/content/tests/test_views.py index eadad7e3..a5867260 100644 --- a/pydis_site/apps/content/tests/test_views.py +++ b/pydis_site/apps/content/tests/test_views.py @@ -1,9 +1,14 @@ +import textwrap from pathlib import Path from unittest import TestCase +import django.test +import markdown from django.http import Http404 from django.test import RequestFactory, SimpleTestCase, override_settings +from django.urls import reverse +from pydis_site.apps.content.models import Tag from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) @@ -180,3 +185,126 @@ class PageOrCategoryViewTests(MockPagesTestCase, SimpleTestCase, TestCase): {"name": PARSED_CATEGORY_INFO["title"], "path": Path("category/subcategory")}, ] ) + + +class TagViewTests(django.test.TestCase): + """Tests for the TagView class.""" + + def setUp(self): + """Set test helpers, then set up fake filesystem.""" + super().setUp() + + def test_valid_tag_returns_200(self): + """Test that a page is returned for a valid tag.""" + Tag.objects.create(name="example", body="This is the tag body.", url="URL") + response = self.client.get("/pages/tags/example/") + self.assertEqual(200, response.status_code) + self.assertIn("This is the tag body", response.content.decode("utf-8")) + self.assertTemplateUsed(response, "content/tag.html") + + def test_invalid_tag_404(self): + """Test that a tag which doesn't exist raises a 404.""" + response = self.client.get("/pages/tags/non-existent/") + self.assertEqual(404, response.status_code) + + def test_context(self): + """Check that the context contains all the necessary data.""" + body = textwrap.dedent(""" + --- + unused: frontmatter + ---- + Tag content here. + """) + + tag = Tag.objects.create(name="example", body=body, url="URL") + response = self.client.get("/pages/tags/example/") + expected = { + "page_title": "example", + "page": markdown.markdown("Tag content here."), + "tag": tag, + } + for key in expected: + self.assertEqual( + expected[key], response.context.get(key), f"context.{key} did not match" + ) + + def test_markdown(self): + """Test that markdown content is rendered properly.""" + body = textwrap.dedent(""" + ```py + Hello world! + ``` + + **This text is in bold** + """) + + Tag.objects.create(name="example", body=body, url="URL") + response = self.client.get("/pages/tags/example/") + content = response.content.decode("utf-8") + + self.assertInHTML('Hello world!', content) + self.assertInHTML("This text is in bold", content) + + def test_embed(self): + """Test that an embed from the frontmatter is treated correctly.""" + body = textwrap.dedent(""" + --- + embed: + title: Embed title + image: + url: https://google.com + --- + Tag body. + """) + + Tag.objects.create(name="example", body=body, url="URL") + response = self.client.get("/pages/tags/example/") + content = response.content.decode("utf-8") + + self.assertInHTML('Embed title', content) + self.assertInHTML("

Tag body.

", content) + + def test_embed_title(self): + """Test that the page title gets set to the embed title.""" + body = textwrap.dedent(""" + --- + embed: + title: Embed title + --- + """) + + Tag.objects.create(name="example", body=body, url="URL") + response = self.client.get("/pages/tags/example/") + self.assertEqual( + "Embed title", + response.context.get("page_title"), + "The page title must match the embed title." + ) + + def test_hyperlinked_item(self): + """Test hyperlinking of tags works as intended.""" + filler_before, filler_after = "empty filler text\n\n", "more\nfiller" + body = filler_before + "`!tags return`" + filler_after + Tag.objects.create(name="example", body=body, url="URL") + + other_url = reverse("content:tag", kwargs={"name": "return"}) + response = self.client.get("/pages/tags/example/") + self.assertEqual( + markdown.markdown(filler_before + f"[`!tags return`]({other_url})" + filler_after), + response.context.get("page") + ) + + def test_tag_root_page(self): + """Test the root tag page which lists all tags.""" + Tag.objects.create(name="tag-1") + Tag.objects.create(name="tag-2") + Tag.objects.create(name="tag-3") + + response = self.client.get("/pages/tags/") + content = response.content.decode("utf-8") + + self.assertTemplateUsed(response, "content/listing.html") + self.assertInHTML('

Tags

', content) + + for tag_number in range(1, 4): + self.assertIn(f"tag-{tag_number}", content) diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index de609596..76437593 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -56,7 +56,7 @@ def fetch_tags() -> list[Tag]: The entire repository is downloaded and extracted locally because getting file content would require one request per file, and can get rate-limited. """ - if settings.GITHUB_TOKEN: + if settings.GITHUB_TOKEN: # pragma: no cover headers = {"Authorization": f"token {settings.GITHUB_TOKEN}"} else: headers = {} @@ -90,7 +90,7 @@ def fetch_tags() -> list[Tag]: def get_tags() -> list[Tag]: """Return a list of all tags visible to the application, from the cache or API.""" - if settings.STATIC_BUILD: + if settings.STATIC_BUILD: # pragma: no cover last_update = None else: last_update = ( @@ -100,7 +100,7 @@ def get_tags() -> list[Tag]: if last_update is None or timezone.now() >= (last_update + TAG_CACHE_TTL): # Stale or empty cache - if settings.STATIC_BUILD: + if settings.STATIC_BUILD: # pragma: no cover tags = get_tags_static() else: tags = fetch_tags() -- cgit v1.2.3 From 74226511e22f04ae5a0f0975849469ad027dc963 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 21:54:46 +0200 Subject: Clean Up Tag Link Substitution Co-authored-by: ChrisJL Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/views/tags.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/views/tags.py b/pydis_site/apps/content/views/tags.py index e2cfb488..5295537d 100644 --- a/pydis_site/apps/content/views/tags.py +++ b/pydis_site/apps/content/views/tags.py @@ -31,11 +31,10 @@ class TagView(TemplateView): content = body[1] # Check for tags which can be hyperlinked - start = 0 - while match := COMMAND_REGEX.search(content, start): + def sub(match: re.Match) -> str: link = reverse("content:tag", kwargs={"name": match.group("name")}) - content = content[:match.start()] + f"[{match.group()}]({link})" + content[match.end():] - start = match.end() + return f"[{match.group()}]({link})" + content = COMMAND_REGEX.sub(sub, content) # Add support for some embed elements if embed := body[0].get("embed"): -- cgit v1.2.3 From f2ad3eed8ef8872713666f69ec783f59006d3d81 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 13 Aug 2022 22:53:50 +0200 Subject: Improve Tag Cropping Move the tag cropping logic to the frontend, which makes it easier to crop without crossing boundaries such as link or code block boundaries. Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/utils.py | 6 +----- pydis_site/static/js/content/listing.js | 36 +++++++++++++++++++++++++++++++ pydis_site/templates/content/listing.html | 4 +++- 3 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 pydis_site/static/js/content/listing.js (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index 76437593..cc08f81f 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -131,14 +131,10 @@ def get_category_pages(path: Path) -> dict[str, dict]: tags = {} for tag in get_tags(): content = frontmatter.parse(tag.body)[1] - if len(content) > 100: - # Trim the preview to a maximum of 100 visible characters - # This causes some markdown to break, but we ignore that - content = content[:100] + "..." tags[tag.name] = { "title": tag.name, - "description": markdown.markdown(content), + "description": markdown.markdown(content, extensions=["pymdownx.superfences"]), "icon": "fas fa-tag" } diff --git a/pydis_site/static/js/content/listing.js b/pydis_site/static/js/content/listing.js new file mode 100644 index 00000000..3502cb2a --- /dev/null +++ b/pydis_site/static/js/content/listing.js @@ -0,0 +1,36 @@ +/** + * Trim a tag listing to only show a few lines of content. + */ +function trimTag() { + const containers = document.getElementsByClassName("tag-container"); + for (const container of containers) { + // Remove every element after the first two paragraphs + while (container.children.length > 2) { + container.removeChild(container.lastChild); + } + + // Trim down the elements if they are too long + const containerLength = container.textContent.length; + if (containerLength > 300) { + if (containerLength - container.firstChild.textContent.length > 300) { + // The first element alone takes up more than 300 characters + container.removeChild(container.lastChild); + } + + let last = container.lastChild.lastChild; + while (container.textContent.length > 300 && container.lastChild.childNodes.length > 0) { + last = container.lastChild.lastChild; + last.remove(); + } + + if (container.textContent.length > 300 && (last instanceof HTMLElement && last.tagName !== "CODE")) { + // Add back the final element (up to a period if possible) + const stop = last.textContent.indexOf("."); + last.textContent = last.textContent.slice(0, stop > 0 ? stop + 1: null); + container.lastChild.appendChild(last); + } + } + } +} + +trimTag(); diff --git a/pydis_site/templates/content/listing.html b/pydis_site/templates/content/listing.html index eeb6b5e2..098f4237 100644 --- a/pydis_site/templates/content/listing.html +++ b/pydis_site/templates/content/listing.html @@ -1,5 +1,6 @@ {# Base navigation screen for resources #} {% extends 'content/base.html' %} +{% load static %} {% block page_content %} {# Nested Categories #} @@ -26,10 +27,11 @@ {{ data.title }} {% if "tags" in location %} -

{{ data.description | safe }}

+
{{ data.description | safe }}
{% else %}

{{ data.description }}

{% endif %} {% endfor %} + {% endblock %} -- cgit v1.2.3 From b4911d03faf8eecf5c4cced6f8036b0b2ef01d58 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 14 Aug 2022 04:44:57 +0200 Subject: Move Tag URL To Property And Add Group The URLs can be simply constructed using the other tag properties, so they were removed from the database in favor of a property. A group field was also added to support tags within groups. Signed-off-by: Hassan Abouelela --- .../migrations/0002_remove_tag_url_tag_group.py | 22 ++++++++++++++++++++++ pydis_site/apps/content/models/tag.py | 17 ++++++++++++++++- pydis_site/apps/content/tests/test_utils.py | 6 +++--- pydis_site/apps/content/tests/test_views.py | 12 ++++++------ 4 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py b/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py new file mode 100644 index 00000000..e59077f0 --- /dev/null +++ b/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py @@ -0,0 +1,22 @@ +# Generated by Django 4.0.6 on 2022-08-13 23:48 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('content', '0001_initial'), + ] + + operations = [ + migrations.RemoveField( + model_name='tag', + name='url', + ), + migrations.AddField( + model_name='tag', + name='group', + field=models.CharField(help_text='The group the tag belongs to.', max_length=50, null=True), + ), + ] diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py index 1437b96a..01264ff1 100644 --- a/pydis_site/apps/content/models/tag.py +++ b/pydis_site/apps/content/models/tag.py @@ -4,6 +4,8 @@ from django.db import models class Tag(models.Model): """A tag from the python-discord server.""" + URL_BASE = "https://github.com/python-discord/bot/tree/main/bot/resources/tags" + last_updated = models.DateTimeField( help_text="The date and time this data was last fetched.", auto_now=True, @@ -13,5 +15,18 @@ class Tag(models.Model): primary_key=True, max_length=50, ) + group = models.CharField( + help_text="The group the tag belongs to.", + null=True, + max_length=50, + ) body = models.TextField(help_text="The content of the tag.") - url = models.URLField(help_text="The URL to this tag on GitHub.") + + @property + def url(self) -> str: + """Get the URL of the tag on GitHub.""" + url = Tag.URL_BASE + if self.group: + url += f"/{self.group}" + url += f"/{self.name}.md" + return url diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index 89ef81c4..a5d5dcb4 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -112,7 +112,7 @@ class TagUtilsTests(TestCase): @mock.patch.object(utils, "fetch_tags") def test_static_fetch(self, fetch_mock: mock.Mock): """Test that the static fetch function is only called at most once during static builds.""" - tags = [models.Tag(name="Name", body="body", url="url")] + tags = [models.Tag(name="Name", body="body")] fetch_mock.return_value = tags result = utils.get_tags_static() second_result = utils.get_tags_static() @@ -159,8 +159,8 @@ class TagUtilsTests(TestCase): result = utils.fetch_tags() self.assertEqual([ - models.Tag(name="first_tag", body=bodies[0], url=f"{utils.TAG_URL_BASE}/first_tag.md"), - models.Tag(name="second_tag", body=bodies[1], url=f"{utils.TAG_URL_BASE}/first_tag.md"), + models.Tag(name="first_tag", body=bodies[0]), + models.Tag(name="second_tag", body=bodies[1]), ], sorted(result, key=lambda tag: tag.name)) def test_get_real_tag(self): diff --git a/pydis_site/apps/content/tests/test_views.py b/pydis_site/apps/content/tests/test_views.py index a5867260..c4d3474e 100644 --- a/pydis_site/apps/content/tests/test_views.py +++ b/pydis_site/apps/content/tests/test_views.py @@ -196,7 +196,7 @@ class TagViewTests(django.test.TestCase): def test_valid_tag_returns_200(self): """Test that a page is returned for a valid tag.""" - Tag.objects.create(name="example", body="This is the tag body.", url="URL") + Tag.objects.create(name="example", body="This is the tag body.") response = self.client.get("/pages/tags/example/") self.assertEqual(200, response.status_code) self.assertIn("This is the tag body", response.content.decode("utf-8")) @@ -216,7 +216,7 @@ class TagViewTests(django.test.TestCase): Tag content here. """) - tag = Tag.objects.create(name="example", body=body, url="URL") + tag = Tag.objects.create(name="example", body=body) response = self.client.get("/pages/tags/example/") expected = { "page_title": "example", @@ -238,7 +238,7 @@ class TagViewTests(django.test.TestCase): **This text is in bold** """) - Tag.objects.create(name="example", body=body, url="URL") + Tag.objects.create(name="example", body=body) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -257,7 +257,7 @@ class TagViewTests(django.test.TestCase): Tag body. """) - Tag.objects.create(name="example", body=body, url="URL") + Tag.objects.create(name="example", body=body) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -273,7 +273,7 @@ class TagViewTests(django.test.TestCase): --- """) - Tag.objects.create(name="example", body=body, url="URL") + Tag.objects.create(name="example", body=body) response = self.client.get("/pages/tags/example/") self.assertEqual( "Embed title", @@ -285,7 +285,7 @@ class TagViewTests(django.test.TestCase): """Test hyperlinking of tags works as intended.""" filler_before, filler_after = "empty filler text\n\n", "more\nfiller" body = filler_before + "`!tags return`" + filler_after - Tag.objects.create(name="example", body=body, url="URL") + Tag.objects.create(name="example", body=body) other_url = reverse("content:tag", kwargs={"name": "return"}) response = self.client.get("/pages/tags/example/") -- cgit v1.2.3 From 45cdb27a82297ede18d7bd908213dde54fef06a9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 14 Aug 2022 05:34:27 +0200 Subject: Add Tag Group Support Adds support for tag groups in content. This involves some modification to the routing, and templating. Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/urls.py | 12 ++- pydis_site/apps/content/utils.py | 101 ++++++++++++++++++++----- pydis_site/apps/content/views/page_category.py | 5 +- pydis_site/apps/content/views/tags.py | 79 +++++++++++++++---- pydis_site/static/css/content/color.css | 7 ++ pydis_site/static/css/content/tag.css | 8 -- pydis_site/static/js/content/listing.js | 5 ++ pydis_site/templates/content/listing.html | 17 ++++- pydis_site/templates/content/tag.html | 5 +- 9 files changed, 187 insertions(+), 52 deletions(-) create mode 100644 pydis_site/static/css/content/color.css (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/urls.py b/pydis_site/apps/content/urls.py index b4ffc07d..03c0015a 100644 --- a/pydis_site/apps/content/urls.py +++ b/pydis_site/apps/content/urls.py @@ -39,15 +39,21 @@ def get_all_pages() -> DISTILL_RETURN: def get_all_tags() -> DISTILL_RETURN: - """Return all tag names in the repository in static builds.""" + """Return all tag names and groups in static builds.""" + groups = {None} for tag in utils.get_tags_static(): - yield {"name": tag.name} + groups.add(tag.group) + yield {"location": (f"{tag.group}/" if tag.group else "") + tag.name} + + groups.remove(None) + for group in groups: + yield {"location": group} urlpatterns = [ distill_path("", views.PageOrCategoryView.as_view(), name='pages'), distill_path( - "tags//", + "tags//", views.TagView.as_view(), name="tag", distill_func=get_all_tags diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index cc08f81f..da6a024d 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -2,6 +2,7 @@ import datetime import functools import tarfile import tempfile +import typing from io import BytesIO from pathlib import Path @@ -16,7 +17,6 @@ from markdown.extensions.toc import TocExtension from pydis_site import settings from .models import Tag -TAG_URL_BASE = "https://github.com/python-discord/bot/tree/main/bot/resources/tags" TAG_CACHE_TTL = datetime.timedelta(hours=1) @@ -44,9 +44,13 @@ def get_tags_static() -> list[Tag]: """ Fetch tag information in static builds. + This also includes some fake tags to preview the tag groups feature. This will return a cached value, so it should only be used for static builds. """ - return fetch_tags() + tags = fetch_tags() + for tag in tags[3:5]: + tag.group = "very-cool-group" + return tags def fetch_tags() -> list[Tag]: @@ -79,10 +83,15 @@ def fetch_tags() -> list[Tag]: repo.extractall(folder, included) for tag_file in Path(folder).rglob("*.md"): + group = None + if tag_file.parent.name != "tags": + # Tags in sub-folders are considered part of a group + group = tag_file.parent.name + tags.append(Tag( name=tag_file.name.removesuffix(".md"), + group=group, body=tag_file.read_text(encoding="utf-8"), - url=f"{TAG_URL_BASE}/{tag_file.name}" )) return tags @@ -114,31 +123,85 @@ def get_tags() -> list[Tag]: return Tag.objects.all() -def get_tag(name: str) -> Tag: - """Return a tag by name.""" - tags = get_tags() - for tag in tags: - if tag.name == name: +def get_tag(path: str) -> typing.Union[Tag, list[Tag]]: + """ + Return a tag based on the search location. + + The tag name and group must match. If only one argument is provided in the path, + it's assumed to either be a group name, or a no-group tag name. + + If it's a group name, a list of tags which belong to it is returned. + """ + path = path.split("/") + if len(path) == 2: + group, name = path[0], path[1] + else: + name = path[0] + group = None + + matches = [] + for tag in get_tags(): + if tag.name == name and tag.group == group: return tag + elif tag.group == name and group is None: + matches.append(tag) + + if matches: + return matches raise Tag.DoesNotExist() -def get_category_pages(path: Path) -> dict[str, dict]: - """Get all page names and their metadata at a category path.""" - # Special handling for tags - if path == Path(__file__).parent / "resources/tags": - tags = {} - for tag in get_tags(): - content = frontmatter.parse(tag.body)[1] +def get_tag_category( + tags: typing.Optional[list[Tag]] = None, *, collapse_groups: bool +) -> dict[str, dict]: + """ + Generate context data for `tags`, or all tags if None. + + If `tags` is None, `get_tag` is used to populate the data. + If `collapse_groups` is True, tags with parent groups are not included in the list, + and instead the parent itself is included as a single entry with it's sub-tags + in the description. + """ + if not tags: + tags = get_tags() + + data = [] + groups = {} - tags[tag.name] = { + # Create all the metadata for the tags + for tag in tags: + if tag.group is None or not collapse_groups: + content = frontmatter.parse(tag.body)[1] + data.append({ "title": tag.name, "description": markdown.markdown(content, extensions=["pymdownx.superfences"]), - "icon": "fas fa-tag" - } + "icon": "fas fa-tag", + }) + else: + if tag.group not in groups: + groups[tag.group] = { + "title": tag.group, + "description": [tag.name], + "icon": "fas fa-tags", + } + else: + groups[tag.group]["description"].append(tag.name) - return {name: tags[name] for name in sorted(tags)} + # Flatten group description into a single string + for group in groups.values(): + group["description"] = "Contains the following tags: " + ", ".join(group["description"]) + data.append(group) + + # Sort the tags, and return them in the proper format + return {tag["title"]: tag for tag in sorted(data, key=lambda tag: tag["title"].lower())} + + +def get_category_pages(path: Path) -> dict[str, dict]: + """Get all page names and their metadata at a category path.""" + # Special handling for tags + if path == Path(__file__).parent / "resources/tags": + return get_tag_category(collapse_groups=True) pages = {} diff --git a/pydis_site/apps/content/views/page_category.py b/pydis_site/apps/content/views/page_category.py index 01ce8402..062c2bc1 100644 --- a/pydis_site/apps/content/views/page_category.py +++ b/pydis_site/apps/content/views/page_category.py @@ -5,7 +5,7 @@ from django.conf import settings from django.http import Http404, HttpRequest, HttpResponse from django.views.generic import TemplateView -from pydis_site.apps.content import utils +from pydis_site.apps.content import models, utils class PageOrCategoryView(TemplateView): @@ -91,4 +91,7 @@ class PageOrCategoryView(TemplateView): "page_title": category["title"], "page_description": category["description"], "icon": category.get("icon"), + "app_name": "content:page_category", + "is_tag_listing": "/resources/tags" in path.as_posix(), + "tag_url": models.Tag.URL_BASE, } diff --git a/pydis_site/apps/content/views/tags.py b/pydis_site/apps/content/views/tags.py index 5295537d..a8df65db 100644 --- a/pydis_site/apps/content/views/tags.py +++ b/pydis_site/apps/content/views/tags.py @@ -1,4 +1,5 @@ import re +import typing import frontmatter import markdown @@ -16,23 +17,65 @@ COMMAND_REGEX = re.compile(r"`*!tags? (?P[\w\d-]+)`*") class TagView(TemplateView): """Handles tag pages.""" - template_name = "content/tag.html" + tag: typing.Union[Tag, list[Tag]] + is_group: bool + + def setup(self, *args, **kwargs) -> None: + """Look for a tag, and configure the view.""" + super().setup(*args, **kwargs) - def get_context_data(self, **kwargs) -> dict: - """Get the relevant context for this tag page.""" try: - tag = utils.get_tag(kwargs.get("name")) + self.tag = utils.get_tag(kwargs.get("location")) + self.is_group = isinstance(self.tag, list) except Tag.DoesNotExist: raise Http404 + def get_template_names(self) -> list[str]: + """Either return the tag page template, or the listing.""" + if self.is_group: + template_name = "content/listing.html" + else: + template_name = "content/tag.html" + + return [template_name] + + def get_context_data(self, **kwargs) -> dict: + """Get the relevant context for this tag page or group.""" context = super().get_context_data(**kwargs) - context["page_title"] = tag.name + context["breadcrumb_items"] = [{ + "name": utils.get_category(settings.CONTENT_PAGES_PATH / location)["title"], + "path": location, + } for location in (".", "tags")] + + if self.is_group: + self._set_group_context(context, self.tag) + else: + self._set_tag_context(context, self.tag) + + return context + + @staticmethod + def _set_tag_context(context: dict[str, any], tag: Tag) -> None: + """Update the context with the information for a tag page.""" + context.update({ + "page_title": tag.name, + "tag": tag, + }) + + if tag.group: + # Add group names to the breadcrumbs + context["breadcrumb_items"].append({ + "name": tag.group, + "path": f"tags/{tag.group}", + }) + + # Clean up tag body body = frontmatter.parse(tag.body) content = body[1] # Check for tags which can be hyperlinked def sub(match: re.Match) -> str: - link = reverse("content:tag", kwargs={"name": match.group("name")}) + link = reverse("content:tag", kwargs={"location": match.group("name")}) return f"[{match.group()}]({link})" content = COMMAND_REGEX.sub(sub, content) @@ -42,14 +85,20 @@ class TagView(TemplateView): if image := embed.get("image"): content = f"![{embed['title']}]({image['url']})\n\n" + content + # Insert the content + context["page"] = markdown.markdown(content, extensions=["pymdownx.superfences"]) + + @staticmethod + def _set_group_context(context: dict[str, any], tags: list[Tag]) -> None: + """Update the context with the information for a group of tags.""" + group = tags[0].group context.update({ - "page": markdown.markdown(content, extensions=["pymdownx.superfences"]), - "tag": tag, + "categories": {}, + "pages": utils.get_tag_category(tags, collapse_groups=False), + "page_title": group, + "icon": "fab fa-tags", + "is_tag_listing": True, + "app_name": "content:tag", + "path": f"{group}/", + "tag_url": f"{tags[0].URL_BASE}/{group}" }) - - context["breadcrumb_items"] = [{ - "name": utils.get_category(settings.CONTENT_PAGES_PATH / location)["title"], - "path": str(location) - } for location in [".", "tags"]] - - return context diff --git a/pydis_site/static/css/content/color.css b/pydis_site/static/css/content/color.css new file mode 100644 index 00000000..f4801c28 --- /dev/null +++ b/pydis_site/static/css/content/color.css @@ -0,0 +1,7 @@ +.content .fa-github { + color: black; +} + +.content .fa-github:hover { + color: #7289DA; +} diff --git a/pydis_site/static/css/content/tag.css b/pydis_site/static/css/content/tag.css index a3db046c..ec45bfc7 100644 --- a/pydis_site/static/css/content/tag.css +++ b/pydis_site/static/css/content/tag.css @@ -1,11 +1,3 @@ -h1.title a { - color: black; -} - -h1.title a:hover { - color: #7289DA; -} - .content a * { /* This is the original color, but propagated down the chain */ /* which allows for elements inside links, such as codeblocks */ diff --git a/pydis_site/static/js/content/listing.js b/pydis_site/static/js/content/listing.js index 3502cb2a..4b722632 100644 --- a/pydis_site/static/js/content/listing.js +++ b/pydis_site/static/js/content/listing.js @@ -4,6 +4,11 @@ function trimTag() { const containers = document.getElementsByClassName("tag-container"); for (const container of containers) { + if (container.textContent.startsWith("Contains the following tags:")) { + // Tag group, no need to trim + continue; + } + // Remove every element after the first two paragraphs while (container.children.length > 2) { container.removeChild(container.lastChild); diff --git a/pydis_site/templates/content/listing.html b/pydis_site/templates/content/listing.html index 098f4237..934b95f6 100644 --- a/pydis_site/templates/content/listing.html +++ b/pydis_site/templates/content/listing.html @@ -2,6 +2,19 @@ {% extends 'content/base.html' %} {% load static %} +{# Show a GitHub button on tag pages #} +{% block title_element %} +{% if is_tag_listing %} + +
+
{{ block.super }}
+
+ +
+
+{% endif %} +{% endblock %} + {% block page_content %} {# Nested Categories #} {% for category, data in categories.items %} @@ -23,10 +36,10 @@ - + {{ data.title }} - {% if "tags" in location %} + {% if is_tag_listing %}
{{ data.description | safe }}
{% else %}

{{ data.description }}

diff --git a/pydis_site/templates/content/tag.html b/pydis_site/templates/content/tag.html index 264f63d0..9bd65744 100644 --- a/pydis_site/templates/content/tag.html +++ b/pydis_site/templates/content/tag.html @@ -3,6 +3,7 @@ {% block head %} {{ block.super }} + {{ tag.name }} {% endblock %} @@ -15,7 +16,3 @@ {% endblock %} - -{% block page_content %} - {{ block.super }} -{% endblock %} -- cgit v1.2.3 From 5aeffa5ab4dd8b251d2ae742d1a1e2bf3ba461c7 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 14 Aug 2022 07:14:50 +0200 Subject: Update Tests For Tag Groups Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/tests/test_utils.py | 110 +++++++++++++++++++++++----- pydis_site/apps/content/tests/test_views.py | 68 ++++++++++++++++- pydis_site/apps/content/utils.py | 3 +- 3 files changed, 157 insertions(+), 24 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index a5d5dcb4..556f633c 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -132,6 +132,7 @@ class TagUtilsTests(TestCase): --- This tag has frontmatter! """), + "This is a grouped tag!", ) # Generate a tar archive with a few tags @@ -146,6 +147,10 @@ class TagUtilsTests(TestCase): (tags_folder / "first_tag.md").write_text(bodies[0]) (tags_folder / "second_tag.md").write_text(bodies[1]) + group_folder = tags_folder / "some_group" + group_folder.mkdir() + (group_folder / "grouped_tag.md").write_text(bodies[2]) + with tarfile.open(tar_folder / "temp.tar", "w") as file: file.add(folder, recursive=True) @@ -158,10 +163,15 @@ class TagUtilsTests(TestCase): ) result = utils.fetch_tags() - self.assertEqual([ + + def sort(_tag: models.Tag) -> str: + return _tag.name + + self.assertEqual(sorted([ models.Tag(name="first_tag", body=bodies[0]), models.Tag(name="second_tag", body=bodies[1]), - ], sorted(result, key=lambda tag: tag.name)) + models.Tag(name="grouped_tag", body=bodies[2], group=group_folder.name), + ], key=sort), sorted(result, key=sort)) def test_get_real_tag(self): """Test that a single tag is returned if it exists.""" @@ -170,30 +180,92 @@ class TagUtilsTests(TestCase): self.assertEqual(tag, result) + def test_get_grouped_tag(self): + """Test fetching a tag from a group.""" + tag = models.Tag.objects.create(name="real-tag", group="real-group") + result = utils.get_tag("real-group/real-tag") + + self.assertEqual(tag, result) + + def test_get_group(self): + """Test fetching a group of tags.""" + included = [ + models.Tag.objects.create(name="tag-1", group="real-group"), + models.Tag.objects.create(name="tag-2", group="real-group"), + models.Tag.objects.create(name="tag-3", group="real-group"), + ] + + models.Tag.objects.create(name="not-included-1") + models.Tag.objects.create(name="not-included-2", group="other-group") + + result = utils.get_tag("real-group") + self.assertListEqual(included, result) + def test_get_tag_404(self): """Test that an error is raised when we fetch a non-existing tag.""" models.Tag.objects.create(name="real-tag") with self.assertRaises(models.Tag.DoesNotExist): utils.get_tag("fake") - def test_category_pages(self): - """Test that the category pages function returns the correct records for tags.""" - models.Tag.objects.create(name="second-tag", body="Normal body") - models.Tag.objects.create(name="first-tag", body="Normal body") - tag_body = {"description": markdown.markdown("Normal body"), "icon": "fas fa-tag"} - + @mock.patch.object(utils, "get_tag_category") + def test_category_pages(self, get_mock: mock.Mock): + """Test that the category pages function calls the correct method for tags.""" + tag = models.Tag.objects.create(name="tag") + get_mock.return_value = tag result = utils.get_category_pages(settings.CONTENT_PAGES_PATH / "tags") + self.assertEqual(tag, result) + get_mock.assert_called_once_with(collapse_groups=True) + + def test_get_category_root(self): + """Test that all tags are returned and formatted properly for the tag root page.""" + body = "normal body" + base = {"description": markdown.markdown(body), "icon": "fas fa-tag"} + + models.Tag.objects.create(name="tag-1", body=body), + models.Tag.objects.create(name="tag-2", body=body), + models.Tag.objects.create(name="tag-3", body=body), + + models.Tag.objects.create(name="tag-4", body=body, group="tag-group") + models.Tag.objects.create(name="tag-5", body=body, group="tag-group") + + result = utils.get_tag_category(collapse_groups=True) + self.assertDictEqual({ - "first-tag": {**tag_body, "title": "first-tag"}, - "second-tag": {**tag_body, "title": "second-tag"}, + "tag-1": {**base, "title": "tag-1"}, + "tag-2": {**base, "title": "tag-2"}, + "tag-3": {**base, "title": "tag-3"}, + "tag-group": { + "title": "tag-group", + "description": "Contains the following tags: tag-4, tag-5", + "icon": "fas fa-tags" + } }, result) - def test_trimmed_tag_content(self): - """Test a tag with a long body that requires trimming.""" - tag = models.Tag.objects.create(name="long-tag", body="E" * 300) - result = utils.get_category_pages(settings.CONTENT_PAGES_PATH / "tags") - self.assertDictEqual({"long-tag": { - "title": "long-tag", - "description": markdown.markdown(tag.body[:100] + "..."), - "icon": "fas fa-tag", - }}, result) + def test_get_category_group(self): + """Test the function for a group root page.""" + body = "normal body" + base = {"description": markdown.markdown(body), "icon": "fas fa-tag"} + + included = [ + models.Tag.objects.create(name="tag-1", body=body, group="group"), + models.Tag.objects.create(name="tag-2", body=body, group="group"), + ] + models.Tag.objects.create(name="not-included", body=body) + + result = utils.get_tag_category(included, collapse_groups=False) + self.assertDictEqual({ + "tag-1": {**base, "title": "tag-1"}, + "tag-2": {**base, "title": "tag-2"}, + }, result) + + def test_tag_url(self): + """Test that tag URLs are generated correctly.""" + cases = [ + ({"name": "tag"}, f"{models.Tag.URL_BASE}/tag.md"), + ({"name": "grouped", "group": "abc"}, f"{models.Tag.URL_BASE}/abc/grouped.md"), + ] + + for options, url in cases: + tag = models.Tag(**options) + with self.subTest(tag=tag): + self.assertEqual(url, tag.url) diff --git a/pydis_site/apps/content/tests/test_views.py b/pydis_site/apps/content/tests/test_views.py index c4d3474e..c5c25be4 100644 --- a/pydis_site/apps/content/tests/test_views.py +++ b/pydis_site/apps/content/tests/test_views.py @@ -194,6 +194,23 @@ class TagViewTests(django.test.TestCase): """Set test helpers, then set up fake filesystem.""" super().setUp() + def test_routing(self): + """Test that the correct template is returned for each route.""" + Tag.objects.create(name="example") + Tag.objects.create(name="grouped-tag", group="group-name") + + cases = [ + ("/pages/tags/example/", "content/tag.html"), + ("/pages/tags/group-name/", "content/listing.html"), + ("/pages/tags/group-name/grouped-tag/", "content/tag.html"), + ] + + for url, template in cases: + with self.subTest(url=url): + response = self.client.get(url) + self.assertEqual(200, response.status_code) + self.assertTemplateUsed(response, template) + def test_valid_tag_returns_200(self): """Test that a page is returned for a valid tag.""" Tag.objects.create(name="example", body="This is the tag body.") @@ -207,8 +224,8 @@ class TagViewTests(django.test.TestCase): response = self.client.get("/pages/tags/non-existent/") self.assertEqual(404, response.status_code) - def test_context(self): - """Check that the context contains all the necessary data.""" + def test_context_tag(self): + """Test that the context contains the required data for a tag.""" body = textwrap.dedent(""" --- unused: frontmatter @@ -222,12 +239,55 @@ class TagViewTests(django.test.TestCase): "page_title": "example", "page": markdown.markdown("Tag content here."), "tag": tag, + "breadcrumb_items": [ + {"name": "Pages", "path": "."}, + {"name": "Tags", "path": "tags"}, + ] } for key in expected: self.assertEqual( expected[key], response.context.get(key), f"context.{key} did not match" ) + def test_context_grouped_tag(self): + """ + Test the context for a tag in a group. + + The only difference between this and a regular tag are the breadcrumbs, + so only those are checked. + """ + Tag.objects.create(name="example", body="Body text", group="group-name") + response = self.client.get("/pages/tags/group-name/example/") + self.assertListEqual([ + {"name": "Pages", "path": "."}, + {"name": "Tags", "path": "tags"}, + {"name": "group-name", "path": "tags/group-name"}, + ], response.context.get("breadcrumb_items")) + + def test_group_page(self): + """Test rendering of a group's root page.""" + Tag.objects.create(name="tag-1", body="Body 1", group="group-name") + Tag.objects.create(name="tag-2", body="Body 2", group="group-name") + Tag.objects.create(name="not-included") + + response = self.client.get("/pages/tags/group-name/") + content = response.content.decode("utf-8") + + self.assertInHTML("
group-name
", content) + self.assertInHTML( + f"", + content + ) + self.assertIn(">tag-1", content) + self.assertIn(">tag-2", content) + self.assertNotIn( + ">not-included", + content, + "Tags not in this group shouldn't be rendered." + ) + + self.assertInHTML("

Body 1

", content) + def test_markdown(self): """Test that markdown content is rendered properly.""" body = textwrap.dedent(""" @@ -287,7 +347,7 @@ class TagViewTests(django.test.TestCase): body = filler_before + "`!tags return`" + filler_after Tag.objects.create(name="example", body=body) - other_url = reverse("content:tag", kwargs={"name": "return"}) + other_url = reverse("content:tag", kwargs={"location": "return"}) response = self.client.get("/pages/tags/example/") self.assertEqual( markdown.markdown(filler_before + f"[`!tags return`]({other_url})" + filler_after), @@ -304,7 +364,7 @@ class TagViewTests(django.test.TestCase): content = response.content.decode("utf-8") self.assertTemplateUsed(response, "content/listing.html") - self.assertInHTML('

Tags

', content) + self.assertInHTML('
Tags
', content) for tag_number in range(1, 4): self.assertIn(f"tag-{tag_number}", content) diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index da6a024d..11100ba5 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -48,7 +48,7 @@ def get_tags_static() -> list[Tag]: This will return a cached value, so it should only be used for static builds. """ tags = fetch_tags() - for tag in tags[3:5]: + for tag in tags[3:5]: # pragma: no cover tag.group = "very-cool-group" return tags @@ -190,6 +190,7 @@ def get_tag_category( # Flatten group description into a single string for group in groups.values(): + # If the following string is updated, make sure to update it in the frontend JS as well group["description"] = "Contains the following tags: " + ", ".join(group["description"]) data.append(group) -- cgit v1.2.3 From f2374900c4c83097c105b56de02ea82d66bd9466 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 21:19:53 +0400 Subject: Unify Tag Migrations & Add Commit Model Signed-off-by: Hassan Abouelela --- .../apps/content/migrations/0001_add_tags.py | 34 ++++++++++++++++++++ pydis_site/apps/content/migrations/0001_initial.py | 23 -------------- .../migrations/0002_remove_tag_url_tag_group.py | 22 ------------- pydis_site/apps/content/models/__init__.py | 4 +-- pydis_site/apps/content/models/tag.py | 36 ++++++++++++++++++++++ 5 files changed, 72 insertions(+), 47 deletions(-) create mode 100644 pydis_site/apps/content/migrations/0001_add_tags.py delete mode 100644 pydis_site/apps/content/migrations/0001_initial.py delete mode 100644 pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/migrations/0001_add_tags.py b/pydis_site/apps/content/migrations/0001_add_tags.py new file mode 100644 index 00000000..2e9d8c45 --- /dev/null +++ b/pydis_site/apps/content/migrations/0001_add_tags.py @@ -0,0 +1,34 @@ +# Generated by Django 4.0.6 on 2022-08-16 16:17 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='Commit', + fields=[ + ('sha', models.CharField(help_text='The SHA hash of this commit.', max_length=40, primary_key=True, serialize=False)), + ('message', models.TextField(help_text='The commit message.')), + ('date', models.DateTimeField(help_text='The date and time the commit was created.')), + ('author', models.TextField(help_text='The person(s) who created the commit.')), + ], + ), + migrations.CreateModel( + name='Tag', + fields=[ + ('last_updated', models.DateTimeField(auto_now=True, help_text='The date and time this data was last fetched.')), + ('name', models.CharField(help_text="The tag's name.", max_length=50, primary_key=True, serialize=False)), + ('group', models.CharField(help_text='The group the tag belongs to.', max_length=50, null=True)), + ('body', models.TextField(help_text='The content of the tag.')), + ('last_commit', models.OneToOneField(help_text='The commit this file was last touched in.', null=True, on_delete=django.db.models.deletion.CASCADE, to='content.commit')), + ], + ), + ] diff --git a/pydis_site/apps/content/migrations/0001_initial.py b/pydis_site/apps/content/migrations/0001_initial.py deleted file mode 100644 index 15e3fc95..00000000 --- a/pydis_site/apps/content/migrations/0001_initial.py +++ /dev/null @@ -1,23 +0,0 @@ -# Generated by Django 4.0.6 on 2022-08-13 00:53 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - initial = True - - dependencies = [ - ] - - operations = [ - migrations.CreateModel( - name='Tag', - fields=[ - ('last_updated', models.DateTimeField(auto_now=True, help_text='The date and time this data was last fetched.')), - ('name', models.CharField(help_text="The tag's name.", max_length=50, primary_key=True, serialize=False)), - ('body', models.TextField(help_text='The content of the tag.')), - ('url', models.URLField(help_text='The URL to this tag on GitHub.')), - ], - ), - ] diff --git a/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py b/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py deleted file mode 100644 index e59077f0..00000000 --- a/pydis_site/apps/content/migrations/0002_remove_tag_url_tag_group.py +++ /dev/null @@ -1,22 +0,0 @@ -# Generated by Django 4.0.6 on 2022-08-13 23:48 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('content', '0001_initial'), - ] - - operations = [ - migrations.RemoveField( - model_name='tag', - name='url', - ), - migrations.AddField( - model_name='tag', - name='group', - field=models.CharField(help_text='The group the tag belongs to.', max_length=50, null=True), - ), - ] diff --git a/pydis_site/apps/content/models/__init__.py b/pydis_site/apps/content/models/__init__.py index 2718ce94..60007e27 100644 --- a/pydis_site/apps/content/models/__init__.py +++ b/pydis_site/apps/content/models/__init__.py @@ -1,3 +1,3 @@ -from .tag import Tag +from .tag import Commit, Tag -__all__ = ["Tag"] +__all__ = ["Commit", "Tag"] diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py index 01264ff1..1c89fe1e 100644 --- a/pydis_site/apps/content/models/tag.py +++ b/pydis_site/apps/content/models/tag.py @@ -1,6 +1,36 @@ +import json + from django.db import models +class Commit(models.Model): + """A git commit.""" + + URL_BASE = "https://github.com/python-discord/bot/commit/" + + sha = models.CharField( + help_text="The SHA hash of this commit.", + primary_key=True, + max_length=40, + ) + message = models.TextField(help_text="The commit message.") + date = models.DateTimeField(help_text="The date and time the commit was created.") + author = models.TextField(help_text="The person(s) who created the commit.") + + @property + def url(self) -> str: + """The URL to the commit on GitHub.""" + return self.URL_BASE + self.sha + + @property + def format_users(self) -> str: + """Return a nice representation of the user(s)' name and email.""" + authors = [] + for author in json.loads(self.author): + authors.append(f"{author['name']} <{author['email']}>") + return ", ".join(authors) + + class Tag(models.Model): """A tag from the python-discord server.""" @@ -10,6 +40,12 @@ class Tag(models.Model): help_text="The date and time this data was last fetched.", auto_now=True, ) + last_commit = models.OneToOneField( + Commit, + help_text="The commit this file was last touched in.", + null=True, + on_delete=models.CASCADE, + ) name = models.CharField( help_text="The tag's name.", primary_key=True, -- cgit v1.2.3 From 04babac2f281487adcddbf1e92d9d028896e086e Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 21:21:59 +0400 Subject: Add Tag Metadata Uses the commit API to obtain tag metadata such as when it was last edited, and by whom. Signed-off-by: Hassan Abouelela --- .../apps/content/migrations/0001_add_tags.py | 5 +- pydis_site/apps/content/models/tag.py | 14 ++- pydis_site/apps/content/utils.py | 124 ++++++++++++++++++--- pydis_site/static/css/content/tag.css | 6 +- pydis_site/templates/content/tag.html | 22 +++- 5 files changed, 146 insertions(+), 25 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/migrations/0001_add_tags.py b/pydis_site/apps/content/migrations/0001_add_tags.py index 2e9d8c45..73525243 100644 --- a/pydis_site/apps/content/migrations/0001_add_tags.py +++ b/pydis_site/apps/content/migrations/0001_add_tags.py @@ -1,4 +1,4 @@ -# Generated by Django 4.0.6 on 2022-08-16 16:17 +# Generated by Django 4.0.6 on 2022-08-16 17:38 import django.db.models.deletion from django.db import migrations, models @@ -25,10 +25,11 @@ class Migration(migrations.Migration): name='Tag', fields=[ ('last_updated', models.DateTimeField(auto_now=True, help_text='The date and time this data was last fetched.')), + ('sha', models.CharField(help_text="The tag's hash, as calculated by GitHub.", max_length=40)), ('name', models.CharField(help_text="The tag's name.", max_length=50, primary_key=True, serialize=False)), ('group', models.CharField(help_text='The group the tag belongs to.', max_length=50, null=True)), ('body', models.TextField(help_text='The content of the tag.')), - ('last_commit', models.OneToOneField(help_text='The commit this file was last touched in.', null=True, on_delete=django.db.models.deletion.CASCADE, to='content.commit')), + ('last_commit', models.ForeignKey(help_text='The commit this file was last touched in.', null=True, on_delete=django.db.models.deletion.CASCADE, to='content.commit')), ], ), ] diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py index 1c89fe1e..3c729768 100644 --- a/pydis_site/apps/content/models/tag.py +++ b/pydis_site/apps/content/models/tag.py @@ -1,3 +1,4 @@ +import collections.abc import json from django.db import models @@ -22,13 +23,10 @@ class Commit(models.Model): """The URL to the commit on GitHub.""" return self.URL_BASE + self.sha - @property - def format_users(self) -> str: + def format_users(self) -> collections.abc.Iterable[str]: """Return a nice representation of the user(s)' name and email.""" - authors = [] for author in json.loads(self.author): - authors.append(f"{author['name']} <{author['email']}>") - return ", ".join(authors) + yield f"{author['name']} <{author['email']}>" class Tag(models.Model): @@ -40,7 +38,11 @@ class Tag(models.Model): help_text="The date and time this data was last fetched.", auto_now=True, ) - last_commit = models.OneToOneField( + sha = models.CharField( + help_text="The tag's hash, as calculated by GitHub.", + max_length=40, + ) + last_commit = models.ForeignKey( Commit, help_text="The commit this file was last touched in.", null=True, diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index 11100ba5..7b078de6 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -1,5 +1,6 @@ import datetime import functools +import json import tarfile import tempfile import typing @@ -15,11 +16,26 @@ from django.utils import timezone from markdown.extensions.toc import TocExtension from pydis_site import settings -from .models import Tag +from .models import Commit, Tag TAG_CACHE_TTL = datetime.timedelta(hours=1) +def github_client(**kwargs) -> httpx.Client: + """Get a client to access the GitHub API with important settings pre-configured.""" + client = httpx.Client( + base_url=settings.GITHUB_API, + follow_redirects=True, + timeout=settings.TIMEOUT_PERIOD, + **kwargs + ) + if settings.GITHUB_TOKEN: # pragma: no cover + if not client.headers.get("Authorization"): + client.headers = {"Authorization": f"token {settings.GITHUB_TOKEN}"} + + return client + + def get_category(path: Path) -> dict[str, str]: """Load category information by name from _info.yml.""" if not path.is_dir(): @@ -60,19 +76,31 @@ def fetch_tags() -> list[Tag]: The entire repository is downloaded and extracted locally because getting file content would require one request per file, and can get rate-limited. """ - if settings.GITHUB_TOKEN: # pragma: no cover - headers = {"Authorization": f"token {settings.GITHUB_TOKEN}"} - else: - headers = {} + client = github_client() + + # Grab metadata + metadata = client.get("/repos/python-discord/bot/contents/bot/resources") + metadata.raise_for_status() + + hashes = {} + for entry in metadata.json(): + if entry["type"] == "dir": + # Tag group + files = client.get(entry["url"]) + files.raise_for_status() + files = files.json() + else: + files = [entry] - tar_file = httpx.get( - f"{settings.GITHUB_API}/repos/python-discord/bot/tarball", - follow_redirects=True, - timeout=settings.TIMEOUT_PERIOD, - headers=headers, - ) + for file in files: + hashes[file["name"]] = file["sha"] + + # Download the files + tar_file = client.get("/repos/python-discord/bot/tarball") tar_file.raise_for_status() + client.close() + tags = [] with tempfile.TemporaryDirectory() as folder: with tarfile.open(fileobj=BytesIO(tar_file.content)) as repo: @@ -83,20 +111,83 @@ def fetch_tags() -> list[Tag]: repo.extractall(folder, included) for tag_file in Path(folder).rglob("*.md"): + name = tag_file.name group = None if tag_file.parent.name != "tags": # Tags in sub-folders are considered part of a group group = tag_file.parent.name tags.append(Tag( - name=tag_file.name.removesuffix(".md"), + name=name.removesuffix(".md"), + sha=hashes[name], group=group, body=tag_file.read_text(encoding="utf-8"), + last_commit=None, )) return tags +def set_tag_commit(tag: Tag) -> Tag: + """Fetch commit information from the API, and save it for the tag.""" + path = "/bot/resources/tags" + if tag.group: + path += f"/{tag.group}" + path += f"/{tag.name}.md" + + # Fetch and set the commit + with github_client() as client: + data = client.get("/repos/python-discord/bot/commits", params={"path": path}) + data.raise_for_status() + data = data.json()[0] + + commit = data["commit"] + author, committer = commit["author"], commit["committer"] + + date = datetime.datetime.strptime(committer["date"], settings.GITHUB_TIMESTAMP_FORMAT) + date = date.replace(tzinfo=datetime.timezone.utc) + + if author["email"] == committer["email"]: + commit_author = [author] + else: + commit_author = [author, committer] + + commit_obj, _ = Commit.objects.get_or_create( + sha=data["sha"], + message=commit["message"], + date=date, + author=json.dumps(commit_author), + ) + tag.last_commit = commit_obj + tag.save() + + return tag + + +def record_tags(tags: list[Tag]) -> None: + """Sync the database with an updated set of tags.""" + # Remove entries which no longer exist + Tag.objects.exclude(name__in=[tag.name for tag in tags]).delete() + + # Insert/update the tags + for tag in tags: + try: + old_tag = Tag.objects.get(name=tag.name) + except Tag.DoesNotExist: + # The tag is not in the database yet, + # pretend it's previous state is the current state + old_tag = tag + + if old_tag.sha == tag.sha and old_tag.last_commit is not None: + # We still have an up-to-date commit entry + tag.last_commit = old_tag.last_commit + + tag.save() + + # Drop old, unused commits + Commit.objects.filter(tag__isnull=True).delete() + + def get_tags() -> list[Tag]: """Return a list of all tags visible to the application, from the cache or API.""" if settings.STATIC_BUILD: # pragma: no cover @@ -113,9 +204,7 @@ def get_tags() -> list[Tag]: tags = get_tags_static() else: tags = fetch_tags() - Tag.objects.exclude(name__in=[tag.name for tag in tags]).delete() - for tag in tags: - tag.save() + record_tags(tags) return tags else: @@ -127,6 +216,9 @@ def get_tag(path: str) -> typing.Union[Tag, list[Tag]]: """ Return a tag based on the search location. + If certain tag data is out of sync (for instance a commit date is missing), + an extra request will be made to sync the information. + The tag name and group must match. If only one argument is provided in the path, it's assumed to either be a group name, or a no-group tag name. @@ -142,6 +234,8 @@ def get_tag(path: str) -> typing.Union[Tag, list[Tag]]: matches = [] for tag in get_tags(): if tag.name == name and tag.group == group: + if tag.last_commit is None: + set_tag_commit(tag) return tag elif tag.group == name and group is None: matches.append(tag) diff --git a/pydis_site/static/css/content/tag.css b/pydis_site/static/css/content/tag.css index 32a605a8..79795f9e 100644 --- a/pydis_site/static/css/content/tag.css +++ b/pydis_site/static/css/content/tag.css @@ -5,5 +5,9 @@ } .content a *:hover { - color: black; + color: dimgray; +} + +span.update-time { + text-decoration: black underline dotted; } diff --git a/pydis_site/templates/content/tag.html b/pydis_site/templates/content/tag.html index 9bd65744..513009da 100644 --- a/pydis_site/templates/content/tag.html +++ b/pydis_site/templates/content/tag.html @@ -9,10 +9,30 @@ {% endblock %} {% block title_element %} -
+ + + {% endblock %} -- cgit v1.2.3 From 7c240c68e24c0f3bf041522ce21de271cb92c6f3 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 22:08:39 +0400 Subject: Better Split Up Tag Commit Messages Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/models/tag.py | 5 +++++ pydis_site/templates/content/tag.html | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py index 3c729768..c504ce21 100644 --- a/pydis_site/apps/content/models/tag.py +++ b/pydis_site/apps/content/models/tag.py @@ -23,6 +23,11 @@ class Commit(models.Model): """The URL to the commit on GitHub.""" return self.URL_BASE + self.sha + def lines(self) -> collections.abc.Iterable[str]: + """Return each line in the commit message.""" + for line in self.message.split("\n"): + yield line + def format_users(self) -> collections.abc.Iterable[str]: """Return a nice representation of the user(s)' name and email.""" for author in json.loads(self.author): diff --git a/pydis_site/templates/content/tag.html b/pydis_site/templates/content/tag.html index 513009da..655dd786 100644 --- a/pydis_site/templates/content/tag.html +++ b/pydis_site/templates/content/tag.html @@ -31,7 +31,9 @@ {% endfor %} - + {% for line in tag.last_commit.lines %} + + {% endfor %}
-- cgit v1.2.3 From 89b0853245fdf5ba7f1f386d7ea7ab1548b538da Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 22:09:14 +0400 Subject: Fix Tag Metadata For Static Builds Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/utils.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index 7b078de6..e4a24a73 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -128,8 +128,19 @@ def fetch_tags() -> list[Tag]: return tags -def set_tag_commit(tag: Tag) -> Tag: +def set_tag_commit(tag: Tag) -> None: """Fetch commit information from the API, and save it for the tag.""" + if settings.STATIC_BUILD: + # Static builds request every page during build, which can ratelimit it. + # Instead, we return some fake data. + tag.last_commit = Commit( + sha="68da80efc00d9932a209d5cccd8d344cec0f09ea", + message="Initial Commit\n\nTHIS IS FAKE DEMO DATA", + date=datetime.datetime(2018, 2, 3, 12, 20, 26, tzinfo=datetime.timezone.utc), + author=json.dumps([{"name": "Joseph", "email": "joseph@josephbanks.me"}]), + ) + return + path = "/bot/resources/tags" if tag.group: path += f"/{tag.group}" @@ -161,8 +172,6 @@ def set_tag_commit(tag: Tag) -> Tag: tag.last_commit = commit_obj tag.save() - return tag - def record_tags(tags: list[Tag]) -> None: """Sync the database with an updated set of tags.""" -- cgit v1.2.3 From 92a42694b6ad1a29e5a21e0b3e57639528837113 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 23:45:25 +0400 Subject: Fix Tests For Tag Metadata Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/tests/test_github_utils.py | 7 +- pydis_site/apps/content/tests/test_utils.py | 132 +++++++++++++++++++++++-- pydis_site/apps/content/tests/test_views.py | 36 ++++--- pydis_site/apps/content/utils.py | 2 +- 4 files changed, 148 insertions(+), 29 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index f642f689..6e25bc80 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -11,6 +11,7 @@ import rest_framework.response import rest_framework.test from django.urls import reverse +from pydis_site import settings from .. import github_utils @@ -49,7 +50,7 @@ class CheckRunTests(unittest.TestCase): "head_sha": "sha", "status": "completed", "conclusion": "success", - "created_at": datetime.datetime.utcnow().strftime(github_utils.ISO_FORMAT_STRING), + "created_at": datetime.datetime.utcnow().strftime(settings.GITHUB_TIMESTAMP_FORMAT), "artifacts_url": "url", } @@ -74,7 +75,7 @@ class CheckRunTests(unittest.TestCase): # to guarantee the right conclusion kwargs["created_at"] = ( datetime.datetime.utcnow() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) - ).strftime(github_utils.ISO_FORMAT_STRING) + ).strftime(settings.GITHUB_TIMESTAMP_FORMAT) with self.assertRaises(github_utils.RunTimeoutError): github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) @@ -178,7 +179,7 @@ class ArtifactFetcherTests(unittest.TestCase): run = github_utils.WorkflowRun( name="action_name", head_sha="action_sha", - created_at=datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + created_at=datetime.datetime.now().strftime(settings.GITHUB_TIMESTAMP_FORMAT), status="completed", conclusion="success", artifacts_url="artifacts_url" diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index 556f633c..2ef033e4 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -1,3 +1,5 @@ +import datetime +import json import tarfile import tempfile import textwrap @@ -15,6 +17,18 @@ from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) +_time = datetime.datetime(2022, 10, 10, 10, 10, 10, tzinfo=datetime.timezone.utc) +_time_str = _time.strftime(settings.GITHUB_TIMESTAMP_FORMAT) +TEST_COMMIT_KWARGS = { + "sha": "123", + "message": "Hello world\n\nThis is a commit message", + "date": _time, + "author": json.dumps([ + {"name": "Author 1", "email": "mail1@example.com", "date": _time_str}, + {"name": "Author 2", "email": "mail2@example.com", "date": _time_str}, + ]), +} + class GetCategoryTests(MockPagesTestCase): """Tests for the get_category function.""" @@ -109,6 +123,10 @@ class GetPageTests(MockPagesTestCase): class TagUtilsTests(TestCase): """Tests for the tag-related utilities.""" + def setUp(self) -> None: + super().setUp() + self.commit = models.Commit.objects.create(**TEST_COMMIT_KWARGS) + @mock.patch.object(utils, "fetch_tags") def test_static_fetch(self, fetch_mock: mock.Mock): """Test that the static fetch function is only called at most once during static builds.""" @@ -121,9 +139,27 @@ class TagUtilsTests(TestCase): self.assertEqual(tags, result) self.assertEqual(tags, second_result) - @mock.patch("httpx.get") + @mock.patch("httpx.Client.get") def test_mocked_fetch(self, get_mock: mock.Mock): """Test that proper data is returned from fetch, but with a mocked API response.""" + fake_request = httpx.Request("GET", "https://google.com") + + # Metadata requests + returns = [httpx.Response( + request=fake_request, + status_code=200, + json=[ + {"type": "file", "name": "first_tag.md", "sha": "123"}, + {"type": "file", "name": "second_tag.md", "sha": "456"}, + {"type": "dir", "name": "some_group", "sha": "789", "url": "/some_group"}, + ] + ), httpx.Response( + request=fake_request, + status_code=200, + json=[{"type": "file", "name": "grouped_tag.md", "sha": "789123"}] + )] + + # Main content request bodies = ( "This is the first tag!", textwrap.dedent(""" @@ -156,33 +192,36 @@ class TagUtilsTests(TestCase): body = (tar_folder / "temp.tar").read_bytes() - get_mock.return_value = httpx.Response( + returns.append(httpx.Response( status_code=200, content=body, - request=httpx.Request("GET", "https://google.com"), - ) + request=fake_request, + )) + get_mock.side_effect = returns result = utils.fetch_tags() def sort(_tag: models.Tag) -> str: return _tag.name self.assertEqual(sorted([ - models.Tag(name="first_tag", body=bodies[0]), - models.Tag(name="second_tag", body=bodies[1]), - models.Tag(name="grouped_tag", body=bodies[2], group=group_folder.name), + models.Tag(name="first_tag", body=bodies[0], sha="123"), + models.Tag(name="second_tag", body=bodies[1], sha="245"), + models.Tag(name="grouped_tag", body=bodies[2], group=group_folder.name, sha="789123"), ], key=sort), sorted(result, key=sort)) def test_get_real_tag(self): """Test that a single tag is returned if it exists.""" - tag = models.Tag.objects.create(name="real-tag") + tag = models.Tag.objects.create(name="real-tag", last_commit=self.commit) result = utils.get_tag("real-tag") self.assertEqual(tag, result) def test_get_grouped_tag(self): """Test fetching a tag from a group.""" - tag = models.Tag.objects.create(name="real-tag", group="real-group") + tag = models.Tag.objects.create( + name="real-tag", group="real-group", last_commit=self.commit + ) result = utils.get_tag("real-group/real-tag") self.assertEqual(tag, result) @@ -269,3 +308,78 @@ class TagUtilsTests(TestCase): tag = models.Tag(**options) with self.subTest(tag=tag): self.assertEqual(url, tag.url) + + @mock.patch("httpx.Client.get") + def test_get_tag_commit(self, get_mock: mock.Mock): + """Test the get commit function with a normal tag.""" + tag = models.Tag.objects.create(name="example") + + authors = json.loads(self.commit.author) + + get_mock.return_value = httpx.Response( + request=httpx.Request("GET", "https://google.com"), + status_code=200, + json=[{ + "sha": self.commit.sha, + "commit": { + "message": self.commit.message, + "author": authors[0], + "committer": authors[1], + } + }] + ) + + result = utils.get_tag(tag.name) + self.assertEqual(tag, result) + + get_mock.assert_called_once() + call_params = get_mock.call_args[1]["params"] + + self.assertEqual({"path": "/bot/resources/tags/example.md"}, call_params) + self.assertEqual(self.commit, models.Tag.objects.get(name=tag.name).last_commit) + + @mock.patch("httpx.Client.get") + def test_get_group_tag_commit(self, get_mock: mock.Mock): + """Test the get commit function with a group tag.""" + tag = models.Tag.objects.create(name="example", group="group-name") + + authors = json.loads(self.commit.author) + authors.pop() + self.commit.author = json.dumps(authors) + self.commit.save() + + get_mock.return_value = httpx.Response( + request=httpx.Request("GET", "https://google.com"), + status_code=200, + json=[{ + "sha": self.commit.sha, + "commit": { + "message": self.commit.message, + "author": authors[0], + "committer": authors[0], + } + }] + ) + + utils.set_tag_commit(tag) + + get_mock.assert_called_once() + call_params = get_mock.call_args[1]["params"] + + self.assertEqual({"path": "/bot/resources/tags/group-name/example.md"}, call_params) + self.assertEqual(self.commit, models.Tag.objects.get(name=tag.name).last_commit) + + @mock.patch.object(utils, "set_tag_commit") + def test_exiting_commit(self, set_commit_mock: mock.Mock): + """Test that a commit is saved when the data has not changed.""" + tag = models.Tag.objects.create(name="tag-name", body="old body", last_commit=self.commit) + + # This is only applied to the object, not to the database + tag.last_commit = None + + utils.record_tags([tag]) + self.assertEqual(self.commit, tag.last_commit) + + result = utils.get_tag("tag-name") + self.assertEqual(tag, result) + set_commit_mock.assert_not_called() diff --git a/pydis_site/apps/content/tests/test_views.py b/pydis_site/apps/content/tests/test_views.py index c5c25be4..658ac2cc 100644 --- a/pydis_site/apps/content/tests/test_views.py +++ b/pydis_site/apps/content/tests/test_views.py @@ -8,10 +8,11 @@ from django.http import Http404 from django.test import RequestFactory, SimpleTestCase, override_settings from django.urls import reverse -from pydis_site.apps.content.models import Tag +from pydis_site.apps.content.models import Commit, Tag from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) +from pydis_site.apps.content.tests.test_utils import TEST_COMMIT_KWARGS from pydis_site.apps.content.views import PageOrCategoryView @@ -193,11 +194,12 @@ class TagViewTests(django.test.TestCase): def setUp(self): """Set test helpers, then set up fake filesystem.""" super().setUp() + self.commit = Commit.objects.create(**TEST_COMMIT_KWARGS) def test_routing(self): """Test that the correct template is returned for each route.""" - Tag.objects.create(name="example") - Tag.objects.create(name="grouped-tag", group="group-name") + Tag.objects.create(name="example", last_commit=self.commit) + Tag.objects.create(name="grouped-tag", group="group-name", last_commit=self.commit) cases = [ ("/pages/tags/example/", "content/tag.html"), @@ -213,7 +215,7 @@ class TagViewTests(django.test.TestCase): def test_valid_tag_returns_200(self): """Test that a page is returned for a valid tag.""" - Tag.objects.create(name="example", body="This is the tag body.") + Tag.objects.create(name="example", body="This is the tag body.", last_commit=self.commit) response = self.client.get("/pages/tags/example/") self.assertEqual(200, response.status_code) self.assertIn("This is the tag body", response.content.decode("utf-8")) @@ -233,7 +235,7 @@ class TagViewTests(django.test.TestCase): Tag content here. """) - tag = Tag.objects.create(name="example", body=body) + tag = Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") expected = { "page_title": "example", @@ -256,7 +258,9 @@ class TagViewTests(django.test.TestCase): The only difference between this and a regular tag are the breadcrumbs, so only those are checked. """ - Tag.objects.create(name="example", body="Body text", group="group-name") + Tag.objects.create( + name="example", body="Body text", group="group-name", last_commit=self.commit + ) response = self.client.get("/pages/tags/group-name/example/") self.assertListEqual([ {"name": "Pages", "path": "."}, @@ -266,9 +270,9 @@ class TagViewTests(django.test.TestCase): def test_group_page(self): """Test rendering of a group's root page.""" - Tag.objects.create(name="tag-1", body="Body 1", group="group-name") - Tag.objects.create(name="tag-2", body="Body 2", group="group-name") - Tag.objects.create(name="not-included") + Tag.objects.create(name="tag-1", body="Body 1", group="group-name", last_commit=self.commit) + Tag.objects.create(name="tag-2", body="Body 2", group="group-name", last_commit=self.commit) + Tag.objects.create(name="not-included", last_commit=self.commit) response = self.client.get("/pages/tags/group-name/") content = response.content.decode("utf-8") @@ -298,7 +302,7 @@ class TagViewTests(django.test.TestCase): **This text is in bold** """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -317,7 +321,7 @@ class TagViewTests(django.test.TestCase): Tag body. """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -333,7 +337,7 @@ class TagViewTests(django.test.TestCase): --- """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") self.assertEqual( "Embed title", @@ -345,7 +349,7 @@ class TagViewTests(django.test.TestCase): """Test hyperlinking of tags works as intended.""" filler_before, filler_after = "empty filler text\n\n", "more\nfiller" body = filler_before + "`!tags return`" + filler_after - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) other_url = reverse("content:tag", kwargs={"location": "return"}) response = self.client.get("/pages/tags/example/") @@ -356,9 +360,9 @@ class TagViewTests(django.test.TestCase): def test_tag_root_page(self): """Test the root tag page which lists all tags.""" - Tag.objects.create(name="tag-1") - Tag.objects.create(name="tag-2") - Tag.objects.create(name="tag-3") + Tag.objects.create(name="tag-1", last_commit=self.commit) + Tag.objects.create(name="tag-2", last_commit=self.commit) + Tag.objects.create(name="tag-3", last_commit=self.commit) response = self.client.get("/pages/tags/") content = response.content.decode("utf-8") diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index e4a24a73..63f1c41c 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -130,7 +130,7 @@ def fetch_tags() -> list[Tag]: def set_tag_commit(tag: Tag) -> None: """Fetch commit information from the API, and save it for the tag.""" - if settings.STATIC_BUILD: + if settings.STATIC_BUILD: # pragma: no cover # Static builds request every page during build, which can ratelimit it. # Instead, we return some fake data. tag.last_commit = Commit( -- cgit v1.2.3 From c0384c626121684ad4e354aeb817fdbd2741fc4f Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 23 Aug 2022 01:09:01 +0400 Subject: Improve Tag Commit Naming Signed-off-by: Hassan Abouelela --- pydis_site/apps/content/models/tag.py | 6 +++--- pydis_site/apps/content/urls.py | 2 +- pydis_site/templates/content/tag.html | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'pydis_site/apps/content') diff --git a/pydis_site/apps/content/models/tag.py b/pydis_site/apps/content/models/tag.py index c504ce21..73d6cb79 100644 --- a/pydis_site/apps/content/models/tag.py +++ b/pydis_site/apps/content/models/tag.py @@ -5,7 +5,7 @@ from django.db import models class Commit(models.Model): - """A git commit.""" + """A git commit from the Python Discord Bot project.""" URL_BASE = "https://github.com/python-discord/bot/commit/" @@ -28,8 +28,8 @@ class Commit(models.Model): for line in self.message.split("\n"): yield line - def format_users(self) -> collections.abc.Iterable[str]: - """Return a nice representation of the user(s)' name and email.""" + def format_authors(self) -> collections.abc.Iterable[str]: + """Return a nice representation of the author(s)' name and email.""" for author in json.loads(self.author): yield f"{author['name']} <{author['email']}>" diff --git a/pydis_site/apps/content/urls.py b/pydis_site/apps/content/urls.py index 03c0015a..163d05bc 100644 --- a/pydis_site/apps/content/urls.py +++ b/pydis_site/apps/content/urls.py @@ -63,5 +63,5 @@ urlpatterns = [ views.PageOrCategoryView.as_view(), name='page_category', distill_func=get_all_pages - ) + ), ] diff --git a/pydis_site/templates/content/tag.html b/pydis_site/templates/content/tag.html index 655dd786..fa9e44f5 100644 --- a/pydis_site/templates/content/tag.html +++ b/pydis_site/templates/content/tag.html @@ -27,7 +27,7 @@