From 6d3285dbc9888d3de235b8a88575e5a19d606b95 Mon Sep 17 00:00:00 2001
From: Gregoo <versatile.mailbox@gmail.com>
Date: Sat, 25 Jan 2025 08:50:04 +0100
Subject: [PATCH 1/3] Move parse_number out of RebrickableSet as it imports way
too much for such a simple function
---
bricktracker/parser.py | 37 +++++++++++++++++++++++++++++++
bricktracker/rebrickable_set.py | 39 ++-------------------------------
bricktracker/wish_list.py | 4 ++--
3 files changed, 41 insertions(+), 39 deletions(-)
create mode 100644 bricktracker/parser.py
diff --git a/bricktracker/parser.py b/bricktracker/parser.py
new file mode 100644
index 0000000..d3602e2
--- /dev/null
+++ b/bricktracker/parser.py
@@ -0,0 +1,37 @@
+from .exceptions import ErrorException
+
+
+# Make sense of string supposed to contain a set ID
+def parse_set(set: str, /) -> str:
+ number, _, version = set.partition('-')
+
+ # Making sure both are integers
+ if version == '':
+ version = 1
+
+ try:
+ number = int(number)
+ except Exception:
+ raise ErrorException('Number "{number}" is not a number'.format(
+ number=number,
+ ))
+
+ try:
+ version = int(version)
+ except Exception:
+ raise ErrorException('Version "{version}" is not a number'.format(
+ version=version,
+ ))
+
+ # Make sure both are positive
+ if number < 0:
+ raise ErrorException('Number "{number}" should be positive'.format(
+ number=number,
+ ))
+
+ if version < 0:
+ raise ErrorException('Version "{version}" should be positive'.format( # noqa: E501
+ version=version,
+ ))
+
+ return '{number}-{version}'.format(number=number, version=version)
diff --git a/bricktracker/rebrickable_set.py b/bricktracker/rebrickable_set.py
index 37e26b3..5a1c41f 100644
--- a/bricktracker/rebrickable_set.py
+++ b/bricktracker/rebrickable_set.py
@@ -7,6 +7,7 @@ from flask import current_app
from .exceptions import ErrorException, NotFoundException
from .instructions import BrickInstructions
+from .parser import parse_set
from .rebrickable import Rebrickable
from .rebrickable_image import RebrickableImage
from .record import BrickRecord
@@ -98,7 +99,7 @@ class RebrickableSet(BrickRecord):
try:
self.socket.auto_progress(message='Parsing set number')
- set = RebrickableSet.parse_number(str(data['set']))
+ set = parse_set(str(data['set']))
self.socket.auto_progress(
message='Set {set}: loading from Rebrickable'.format(
@@ -187,39 +188,3 @@ class RebrickableSet(BrickRecord):
'url': str(data['set_url']),
'last_modified': str(data['last_modified_dt']),
}
-
- # Make sense of the number from the data
- @staticmethod
- def parse_number(set: str, /) -> str:
- number, _, version = set.partition('-')
-
- # Making sure both are integers
- if version == '':
- version = 1
-
- try:
- number = int(number)
- except Exception:
- raise ErrorException('Number "{number}" is not a number'.format(
- number=number,
- ))
-
- try:
- version = int(version)
- except Exception:
- raise ErrorException('Version "{version}" is not a number'.format(
- version=version,
- ))
-
- # Make sure both are positive
- if number < 0:
- raise ErrorException('Number "{number}" should be positive'.format(
- number=number,
- ))
-
- if version < 0:
- raise ErrorException('Version "{version}" should be positive'.format( # noqa: E501
- version=version,
- ))
-
- return '{number}-{version}'.format(number=number, version=version)
diff --git a/bricktracker/wish_list.py b/bricktracker/wish_list.py
index dfba800..880021b 100644
--- a/bricktracker/wish_list.py
+++ b/bricktracker/wish_list.py
@@ -4,9 +4,9 @@ from typing import Self
from flask import current_app
from .exceptions import NotFoundException
+from .parser import parse_set
from .rebrickable import Rebrickable
from .rebrickable_image import RebrickableImage
-from .rebrickable_set import RebrickableSet
from .record_list import BrickRecordList
from .wish import BrickWish
@@ -34,7 +34,7 @@ class BrickWishList(BrickRecordList[BrickWish]):
@staticmethod
def add(set: str, /) -> None:
try:
- set = RebrickableSet.parse_number(set)
+ set = parse_set(set)
BrickWish().select_specific(set)
except NotFoundException:
logger.debug('rebrick.lego.get_set("{set}")'.format(
From ed44fb9bab81bed7517451b31b57a0c94c87227f Mon Sep 17 00:00:00 2001
From: Gregoo <versatile.mailbox@gmail.com>
Date: Sat, 25 Jan 2025 08:51:18 +0100
Subject: [PATCH 2/3] Global cleanup of the code, implementing all the comments
for the issue
---
bricktracker/config.py | 4 +-
bricktracker/instructions.py | 177 ++++++++++++++++-----------
bricktracker/views/instructions.py | 54 ++++----
templates/instructions/download.html | 7 +-
templates/set/card.html | 4 +-
5 files changed, 138 insertions(+), 108 deletions(-)
diff --git a/bricktracker/config.py b/bricktracker/config.py
index 36defc6..08db61b 100644
--- a/bricktracker/config.py
+++ b/bricktracker/config.py
@@ -43,8 +43,8 @@ CONFIG: Final[list[dict[str, Any]]] = [
{'n': 'REBRICKABLE_IMAGE_NIL_MINIFIGURE', 'd': 'https://rebrickable.com/static/img/nil_mf.jpg'}, # noqa: E501
{'n': 'REBRICKABLE_LINK_MINIFIGURE_PATTERN', 'd': 'https://rebrickable.com/minifigs/{number}'}, # noqa: E501
{'n': 'REBRICKABLE_LINK_PART_PATTERN', 'd': 'https://rebrickable.com/parts/{number}/_/{color}'}, # noqa: E501
- {'n': 'REBRICKABLE_LINK_INSTRUCTIONS_PATTERN', 'd': 'https://rebrickable.com/instructions/{number}'}, # noqa: E501
- {'n': 'REBRICKABLE_USER_AGENT', 'd': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36'},
+ {'n': 'REBRICKABLE_LINK_INSTRUCTIONS_PATTERN', 'd': 'https://rebrickable.com/instructions/{path}'}, # noqa: E501
+ {'n': 'REBRICKABLE_USER_AGENT', 'd': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36'}, # noqa: E501
{'n': 'REBRICKABLE_LINKS', 'e': 'LINKS', 'c': bool},
{'n': 'REBRICKABLE_PAGE_SIZE', 'd': 100, 'c': int},
{'n': 'RETIRED_SETS_FILE_URL', 'd': 'https://docs.google.com/spreadsheets/d/1rlYfEXtNKxUOZt2Mfv0H17DvK7bj6Pe0CuYwq6ay8WA/gviz/tq?tqx=out:csv&sheet=Sorted%20by%20Retirement%20Date'}, # noqa: E501
diff --git a/bricktracker/instructions.py b/bricktracker/instructions.py
index 0c966d3..0dd7f8e 100644
--- a/bricktracker/instructions.py
+++ b/bricktracker/instructions.py
@@ -1,18 +1,18 @@
from datetime import datetime, timezone
import logging
import os
-from typing import TYPE_CHECKING
+from shutil import copyfileobj
+from typing import Tuple, TYPE_CHECKING
-from flask import current_app, g, url_for, flash
+from bs4 import BeautifulSoup
+from flask import current_app, g, url_for
import humanize
+import requests
from werkzeug.datastructures import FileStorage
from werkzeug.utils import secure_filename
-from io import BytesIO
-import requests
-from bs4 import BeautifulSoup
-
-from .exceptions import ErrorException
+from .exceptions import ErrorException, DownloadException
+from .parser import parse_set
if TYPE_CHECKING:
from .rebrickable_set import RebrickableSet
@@ -71,6 +71,34 @@ class BrickInstructions(object):
def delete(self, /) -> None:
os.remove(self.path())
+ # Download an instruction file
+ def download(self, path: str, /) -> None:
+ target = self.path(filename=secure_filename(self.filename))
+
+ if os.path.isfile(target):
+ raise ErrorException('Cannot download {target} as it already exists'.format( # noqa: E501
+ target=self.filename
+ ))
+
+ url = current_app.config['REBRICKABLE_LINK_INSTRUCTIONS_PATTERN'].format( # noqa: E501
+ path=path
+ )
+
+ response = requests.get(url, stream=True)
+ if response.ok:
+ with open(target, 'wb') as f:
+ copyfileobj(response.raw, f)
+ else:
+ raise DownloadException('Failed to download {file}. Status code: {code}'.format( # noqa: E501
+ file=self.filename,
+ code=response.status_code
+ ))
+
+ # Info
+ logger.info('The instruction file {file} has been downloaded'.format(
+ file=self.filename
+ ))
+
# Display the size in a human format
def human_size(self) -> str:
return humanize.naturalsize(self.size)
@@ -118,73 +146,6 @@ class BrickInstructions(object):
))
file.save(target)
-
- # Info
- logger.info('The instruction file {file} has been imported'.format(
- file=self.filename
- ))
-
- # Compute the url for the rebrickable instructions page
- def url_for_instructions(self, /) -> str:
- try:
- return current_app.config['REBRICKABLE_LINK_INSTRUCTIONS_PATTERN'].format( # noqa: E501
- number=self.filename,
- )
- except Exception:
- pass
-
- return ''
-
- def find_instructions(self, set: str, /) -> None:
-
- headers = {
- 'User-Agent': current_app.config['REBRICKABLE_USER_AGENT']
- }
-
- response = requests.get(BrickInstructions.url_for_instructions(self), headers=headers)
- if response.status_code != 200:
- raise ErrorException('Failed to load page. Status code: {response.status_code}')
-
- # Parse the HTML content
- soup = BeautifulSoup(response.content, 'html.parser')
-
- # Collect all <img> tags with "LEGO Building Instructions" in the alt attribute
- found_tags = []
- for a_tag in soup.find_all('a', href=True):
- img_tag = a_tag.find('img', alt=True)
- if img_tag and "LEGO Building Instructions" in img_tag['alt']:
- found_tags.append((img_tag['alt'].replace('LEGO Building Instructions for ', ''), a_tag['href'])) # Save alt and href
-
- return found_tags
-
- def get_list(self, request_form, /) -> list:
- selected_instructions = []
- # Get the list of instructions
- for key in request_form:
- if key.startswith('instruction-') and request_form.get(key) == 'on': # Checkbox is checked
- index = key.split('-')[-1]
- alt_text = request_form.get(f'instruction-alt-text-{index}')
- href_text = request_form.get(f'instruction-href-text-{index}').replace('/instructions/', '') # Remove the /instructions/ part
- selected_instructions.append((href_text,alt_text))
-
- return selected_instructions
-
- def download(self, href: str, /) -> None:
- target = self.path(filename=secure_filename(self.filename))
-
- if os.path.isfile(target):
- raise ErrorException('Cannot download {target} as it already exists'.format( # noqa: E501
- target=self.filename
- ))
-
- url = current_app.config['REBRICKABLE_LINK_INSTRUCTIONS_PATTERN'].format(number=href)
-
- response = requests.get(url)
- if response.status_code == 200:
- # Save the content to the target path
- FileStorage(stream=BytesIO(response.content)).save(target)
- else:
- raise ErrorException(f"Failed to download {self.filename}. Status code: {response.status_code}")
# Info
logger.info('The instruction file {file} has been imported'.format(
@@ -213,3 +174,71 @@ class BrickInstructions(object):
return 'file-image-line'
else:
return 'file-line'
+
+ # Download selected instructions for a set
+ @staticmethod
+ def download_instructions(form: dict[str, str], /) -> None:
+ selected_instructions: list[Tuple[str, str]] = []
+
+ # Get the list of instructions
+ for key in form:
+ if key.startswith('instruction-') and form.get(key) == 'on':
+ _, _, index = key.partition('-')
+ alt_text = form.get(f'instruction-alt-text-{index}', '')
+ href_text = form.get(f'instruction-href-text-{index}', '').removeprefix('/instructions/') # Remove the /instructions/ part # noqa: E501
+ selected_instructions.append((href_text, alt_text))
+
+ # Raise if nothing selected
+ if not len(selected_instructions):
+ raise ErrorException('No instruction was selected to download')
+
+ # Loop over selected instructions and download them
+ for href, filename in selected_instructions:
+ BrickInstructions(f"{filename}.pdf").download(href)
+
+ # Find the instructions for a set
+ @staticmethod
+ def find_instructions(form: dict[str, str], /) -> list[Tuple[str, str]]:
+ # Grab the set ID
+ set: str = form.get('add-set', '')
+
+ # Parse it
+ set = parse_set(set)
+
+ response = requests.get(
+ current_app.config['REBRICKABLE_LINK_INSTRUCTIONS_PATTERN'].format(
+ path=set,
+ ),
+ headers={
+ 'User-Agent': current_app.config['REBRICKABLE_USER_AGENT']
+ }
+ )
+
+ if not response.ok:
+ raise ErrorException('Failed to load the Rebrickable instructions page. Status code: {code}'.format( # noqa: E501
+ code=response.status_code
+ ))
+
+ # Parse the HTML content
+ soup = BeautifulSoup(response.content, 'html.parser')
+
+ # Collect all <img> tags with "LEGO Building Instructions" in the
+ # alt attribute
+ found_tags: list[Tuple[str, str]] = []
+ for a_tag in soup.find_all('a', href=True):
+ img_tag = a_tag.find('img', alt=True)
+ if img_tag and "LEGO Building Instructions" in img_tag['alt']:
+ found_tags.append(
+ (
+ img_tag['alt'].removeprefix('LEGO Building Instructions for '), # noqa: E501
+ a_tag['href']
+ )
+ ) # Save alt and href
+
+ # Raise an error if nothing found
+ if not len(found_tags):
+ raise ErrorException('No instruction found for set {set}'.format(
+ set=set
+ ))
+
+ return found_tags
diff --git a/bricktracker/views/instructions.py b/bricktracker/views/instructions.py
index 585a981..d347b06 100644
--- a/bricktracker/views/instructions.py
+++ b/bricktracker/views/instructions.py
@@ -4,8 +4,7 @@ from flask import (
redirect,
render_template,
request,
- url_for,
- flash
+ url_for
)
from flask_login import login_required
from werkzeug.wrappers.response import Response
@@ -14,6 +13,7 @@ from werkzeug.utils import secure_filename
from .exceptions import exception_handler
from ..instructions import BrickInstructions
from ..instructions_list import BrickInstructionsList
+from ..parser import parse_set
from .upload import upload_helper
instructions_page = Blueprint(
@@ -134,40 +134,38 @@ def do_upload() -> Response:
@login_required
@exception_handler(__file__)
def download() -> str:
+ # Grab the set number
+ try:
+ set = parse_set(request.args.get('set', ''))
+ except Exception:
+ set = ''
+
return render_template(
'instructions.html',
download=True,
- error=request.args.get('error')
+ error=request.args.get('error'),
+ set=set
)
-
+
+
# Show search results
-@instructions_page.route('/download/', methods=['POST'])
+@instructions_page.route('/download/select', methods=['POST'])
+@login_required
+@exception_handler(__file__, post_redirect='instructions.download')
+def select_download() -> str:
+ return render_template(
+ 'instructions.html',
+ download=True,
+ instructions=BrickInstructions.find_instructions(request.form)
+ )
+
+
+# Download files
+@instructions_page.route('/download', methods=['POST'])
@login_required
@exception_handler(__file__, post_redirect='instructions.download')
def do_download() -> Response:
- # get set_id from input field
- set_id: str = request.form.get('add-set', '')
-
- # get list of instructions for the set and offer them to download
- instructions = BrickInstructions(set_id).find_instructions(set_id)
-
- return render_template('instructions.html', download=True, instructions=instructions)
-
-@instructions_page.route('/confirm_download', methods=['POST'])
-@login_required
-@exception_handler(__file__, post_redirect='instructions.download')
-def confirm_download() -> Response:
-
- # Get list of selected instructions
- selected_instructions = BrickInstructions("").get_list(request.form)
-
- # No instructions selected
- if not selected_instructions:
- return redirect(url_for('instructions.download'))
-
- # Loop over selected instructions and download them
- for href, filename in selected_instructions:
- BrickInstructions(f"{filename}.pdf").download(href)
+ BrickInstructions.download_instructions(request.form)
BrickInstructionsList(force=True)
diff --git a/templates/instructions/download.html b/templates/instructions/download.html
index 20cb688..9543702 100644
--- a/templates/instructions/download.html
+++ b/templates/instructions/download.html
@@ -1,7 +1,8 @@
<div class="container">
+ {% if error %}<div class="alert alert-danger" role="alert"><strong>Error:</strong> {{ error }}.</div>{% endif %}
<div class="row">
<div class="col-12">
- <form method="POST" action="{{ url_for('instructions.do_download') }}">
+ <form method="POST" action="{{ url_for('instructions.select_download') }}">
<div class="card mb-3">
<div class="card-header">
<h5 class="mb-0"><i class="ri-add-circle-line"></i> Download instructions from Rebrickable</h5>
@@ -11,7 +12,7 @@
<div id="add-complete" class="alert alert-success d-none" role="alert"></div>
<div class="mb-3">
<label for="add-set" class="form-label">Set number (only one)</label>
- <input type="text" class="form-control" id="add-set" name="add-set" placeholder="107-1 or 1642-1 or ..." value="{{ request.args.get('set_num', '') }}">
+ <input type="text" class="form-control" id="add-set" name="add-set" placeholder="107-1 or 1642-1 or ..." value="{{ set }}">
</div>
<button type="submit" class="btn btn-primary">Search</button>
</div>
@@ -23,7 +24,7 @@
<h5 class="mb-0"><i class="ri-add-circle-line"></i> Select instructions to download</h5>
</div>
<div class="card-body">
- <form method="POST" action="{{ url_for('instructions.confirm_download') }}">
+ <form method="POST" action="{{ url_for('instructions.do_download') }}">
<div class="mb-3">
<label class="form-label">Available Instructions</label>
<div class="form-check">
diff --git a/templates/set/card.html b/templates/set/card.html
index 1fe7f72..9fcea3d 100644
--- a/templates/set/card.html
+++ b/templates/set/card.html
@@ -48,9 +48,11 @@
<span class="list-group-item list-group-item-action text-center"><i class="ri-error-warning-line"></i> No instructions file found.</span>
{% if g.login.is_authenticated() %}
<a class="list-group-item list-group-item-action" href="{{ url_for('instructions.upload') }}"><i class="ri-upload-line"></i> Upload an instructions file</a>
- <a class="list-group-item list-group-item-action" href="{{ url_for('instructions.download', set_num=item.fields.number) }}"><i class="ri-download-line"></i> Download instruction from Rebrickable</a>
{% endif %}
{% endif %}
+ {% if g.login.is_authenticated() %}
+ <a class="list-group-item list-group-item-action" href="{{ url_for('instructions.download', set=item.fields.set) }}"><i class="ri-download-line"></i> Download instructions from Rebrickable</a>
+ {% endif %}
</div>
{{ accordion.footer() }}
{{ accordion.table(item.parts(), 'Parts', 'parts-inventory', 'set-details', 'part/table.html', icon='shapes-line')}}
From fd38e0a1500033f10e6d0ac19b3ba62964da181b Mon Sep 17 00:00:00 2001
From: Gregoo <versatile.mailbox@gmail.com>
Date: Sat, 25 Jan 2025 08:55:42 +0100
Subject: [PATCH 3/3] Fix the default value
---
.env.sample | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.env.sample b/.env.sample
index 51845fe..7cbc5c0 100644
--- a/.env.sample
+++ b/.env.sample
@@ -179,7 +179,7 @@
# BK_REBRICKABLE_LINK_PART_PATTERN=
# Optional: Pattern of the link to Rebrickable for instructions. Will be passed to Python .format()
-# Default: https://rebrickable.com/instructions/{number}
+# Default: https://rebrickable.com/instructions/{path}
# BK_REBRICKABLE_LINK_INSTRUCTIONS_PATTERN=
# Optional: User-Agent to use when querying Rebrickable outside of the Rebrick python library