From e07da529f1f1f337fb751c3775117c53f0af5e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 2 Jul 2019 01:28:02 +0200 Subject: [PATCH] Preview website title + description in bookmark form Fix unnecessary selects when rendering bookmarks --- .idea/misc.xml | 3 + bookmarks/models.py | 8 +- bookmarks/queries.py | 7 +- bookmarks/services/bookmarks.py | 25 +----- bookmarks/services/website_loader.py | 37 +++++++++ bookmarks/styles/bookmarks.scss | 7 ++ bookmarks/templates/bookmarks/form.html | 105 ++++++++++++++++-------- bookmarks/urls.py | 3 + bookmarks/views/api.py | 9 ++ 9 files changed, 145 insertions(+), 59 deletions(-) create mode 100644 bookmarks/services/website_loader.py create mode 100644 bookmarks/views/api.py diff --git a/.idea/misc.xml b/.idea/misc.xml index d09bbac..ad2d41e 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -4,6 +4,9 @@ /usr/local/bin/ghc /usr/local/bin/stack + + diff --git a/bookmarks/models.py b/bookmarks/models.py index 0d5c3c7..964f307 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -42,8 +42,9 @@ class Bookmark(models.Model): tags = models.ManyToManyField(Tag) # Attributes might be calculated in query - tag_count = 0 - tag_string = '' + tag_count = 0 # Projection for number of associated tags + tag_string = '' # Projection for list of tag names, comma-separated + tag_projection = False # Tracks if the above projections were loaded @property def resolved_title(self): @@ -55,7 +56,8 @@ class Bookmark(models.Model): @property def tag_names(self): - if self.tag_string: + # If tag projections were loaded then avoid querying all tags (=executing further selects) + if self.tag_string or self.tag_projection: return parse_tag_string(self.tag_string) else: return [tag.name for tag in self.tags.all()] diff --git a/bookmarks/queries.py b/bookmarks/queries.py index 7450b30..de27a25 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -1,5 +1,5 @@ from django.contrib.auth.models import User -from django.db.models import Q, Count, Aggregate, CharField +from django.db.models import Q, Count, Aggregate, CharField, Value, BooleanField from bookmarks.models import Bookmark, Tag @@ -20,7 +20,8 @@ def query_bookmarks(user: User, query_string: str): # Add aggregated tag info to bookmark instances query_set = Bookmark.objects \ .annotate(tag_count=Count('tags'), - tag_string=Concat('tags__name')) + tag_string=Concat('tags__name'), + tag_projection=Value(True, BooleanField())) # Filter for user query_set = query_set.filter(owner=user) @@ -49,7 +50,7 @@ def query_bookmarks(user: User, query_string: str): def query_tags(user: User, query_string: str): - query_set = Tag.objects; + query_set = Tag.objects # Filter for user query_set = query_set.filter(owner=user) diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 3e6c6d6..f440b90 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -1,10 +1,9 @@ -import requests -from bs4 import BeautifulSoup from django.contrib.auth.models import User from django.utils import timezone from bookmarks.models import Bookmark, BookmarkForm, parse_tag_string from services.tags import get_or_create_tags +from services.website_loader import load_website_metadata def create_bookmark(form: BookmarkForm, current_user: User): @@ -34,28 +33,12 @@ def update_bookmark(form: BookmarkForm, current_user: User): def _update_website_metadata(bookmark: Bookmark): - # noinspection PyBroadException - try: - page_text = load_page(bookmark.url) - soup = BeautifulSoup(page_text, 'html.parser') - - title = soup.title.string if soup.title is not None else None - description_tag = soup.find('meta', attrs={'name': 'description'}) - description = description_tag['content'] if description_tag is not None else None - - bookmark.website_title = title - bookmark.website_description = description - except Exception: - bookmark.website_title = None - bookmark.website_description = None + metadata = load_website_metadata(bookmark.url) + bookmark.website_title = metadata.title + bookmark.website_description = metadata.description def _update_bookmark_tags(bookmark: Bookmark, tag_string: str, user: User): tag_names = parse_tag_string(tag_string, ' ') tags = get_or_create_tags(tag_names, user) bookmark.tags.set(tags) - - -def load_page(url: str): - r = requests.get(url) - return r.text diff --git a/bookmarks/services/website_loader.py b/bookmarks/services/website_loader.py new file mode 100644 index 0000000..5d00f6d --- /dev/null +++ b/bookmarks/services/website_loader.py @@ -0,0 +1,37 @@ +from dataclasses import dataclass + +import requests +from bs4 import BeautifulSoup + + +@dataclass +class WebsiteMetadata: + url: str + title: str + description: str + + def to_dict(self): + return { + 'url': self.url, + 'title': self.title, + 'description': self.description, + } + + +def load_website_metadata(url: str): + title = None + description = None + try: + page_text = load_page(url) + soup = BeautifulSoup(page_text, 'html.parser') + + title = soup.title.string if soup.title is not None else None + description_tag = soup.find('meta', attrs={'name': 'description'}) + description = description_tag['content'] if description_tag is not None else None + finally: + return WebsiteMetadata(url=url, title=title, description=description) + + +def load_page(url: str): + r = requests.get(url) + return r.text diff --git a/bookmarks/styles/bookmarks.scss b/bookmarks/styles/bookmarks.scss index 1e02f17..06f789e 100644 --- a/bookmarks/styles/bookmarks.scss +++ b/bookmarks/styles/bookmarks.scss @@ -49,3 +49,10 @@ ul.bookmark-list { color: $alternative-color-dark; } } + +.bookmarks-form { + + .form-icon.loading { + visibility: hidden; + } +} diff --git a/bookmarks/templates/bookmarks/form.html b/bookmarks/templates/bookmarks/form.html index 1ff922e..bede7c9 100644 --- a/bookmarks/templates/bookmarks/form.html +++ b/bookmarks/templates/bookmarks/form.html @@ -1,40 +1,81 @@ {% load widget_tweaks %} -{% csrf_token %} -
- - {{ form.url|add_class:"form-input"|attr:"autofocus" }} - {% if form.url.errors %} +
+ {% csrf_token %} +
+ + {{ form.url|add_class:"form-input"|attr:"autofocus" }} + {% if form.url.errors %} +
+ {{ form.url.errors }} +
+ {% endif %} +
+
+ + {{ form.tag_string|add_class:"form-input" }}
- {{ form.url.errors }} + Enter any number of tags separated by space and without the hash (#). If a tag does not exist it will be + automatically created.
- {% endif %} -
-
- - {{ form.tag_string|add_class:"form-input" }} -
- Enter any number of tags separated by space and without the hash (#). If a tag does not exist it will be automatically created. + {{ form.tag_string.errors }}
- {{ form.tag_string.errors }} -
-
- - {{ form.title|add_class:"form-input" }} -
- Optional, leave empty to use title from website. +
+ +
+ {{ form.title|add_class:"form-input" }} + +
+
+ Optional, leave empty to use title from website. +
+ {{ form.title.errors }}
- {{ form.title.errors }} -
-
- - {{ form.description|add_class:"form-input"|attr:"rows:4" }} -
- Optional, leave empty to use description from website. +
+ +
+ {{ form.description|add_class:"form-input"|attr:"rows:4" }} + +
+
+ Optional, leave empty to use description from website. +
+ {{ form.description.errors }}
- {{ form.description.errors }} -
-
- - Nevermind +
+ + Nevermind +
+ +
diff --git a/bookmarks/urls.py b/bookmarks/urls.py index 8d2d334..99fc1ad 100644 --- a/bookmarks/urls.py +++ b/bookmarks/urls.py @@ -3,6 +3,7 @@ from django.urls import path from django.views.generic import RedirectView from . import views +from .views import api app_name = 'bookmarks' urlpatterns = [ @@ -13,4 +14,6 @@ urlpatterns = [ path('bookmarks/new', views.new, name='new'), path('bookmarks//edit', views.edit, name='edit'), path('bookmarks//remove', views.remove, name='remove'), + # API + path('api/website_metadata', api.website_metadata, name='api.website_metadata'), ] diff --git a/bookmarks/views/api.py b/bookmarks/views/api.py new file mode 100644 index 0000000..fb8313e --- /dev/null +++ b/bookmarks/views/api.py @@ -0,0 +1,9 @@ +from django.http import JsonResponse + +from services.website_loader import load_website_metadata + + +def website_metadata(request): + url = request.GET.get('url') + metadata = load_website_metadata(url) + return JsonResponse(metadata.to_dict())