Merge pull request 'Bugfix/issue 171' (#174) from bugfix/issue-171 into master

Reviewed-on: #174

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 1.4.2
+
+### Bug Fixes
+
+- **Fixed instructions downloads failing with "No instructions found on Rebrickable or Peeron"** (#171): Rebrickable now blocks scraping behind a Cloudflare challenge, so instructions are fetched from LEGO's own official building-instructions page instead
+- **Fixed instructions never linking to sets with alphanumeric ids** (#166): Instructions for "Pick a Brick" / promotional builds whose set id begins with letters (e.g. `EG00029-1`) were orphaned and never associated with the set; the matching now handles alphanumeric set ids
+
 ## 1.4.1
 
 ### Enhancements

bricktracker/instructions.py

@@ -75,10 +75,11 @@ class BrickInstructions(object):
 
             splits = normalized.split('-', 2)
 
-            if len(splits) >= 2:
+            if len(splits) >= 2 and splits[0]:
                 try:
-                    # Trying to make sense of each part as integers
+                    # The set number can be alphanumeric (e.g. Pick-a-Brick
-                    int(splits[0])
+                    # builds like "EG00029-1"); only the version suffix has
+                    # to be an integer
                     int(splits[1])
 
                     self.set = '-'.join(splits[:2])
@@ -120,18 +121,6 @@ class BrickInstructions(object):
                 'Cache-Control': 'max-age=0'
             })
 
-            # Visit the set's instructions listing page first to establish session cookies
-            set_number = None
-            if self.rebrickable:
-                set_number = self.rebrickable.fields.set
-            elif self.set:
-                set_number = self.set
-
-            if set_number:
-                instructions_page = f"https://rebrickable.com/instructions/{set_number}/"
-                session.get(instructions_page)
-                session.headers.update({"Referer": instructions_page})
-
             resp = session.get(path, stream=True, allow_redirects=True)
             if not resp.ok:
                 raise DownloadException(f"Failed to download: HTTP {resp.status_code}")
@@ -269,10 +258,13 @@ class BrickInstructions(object):
     @staticmethod
     def find_instructions(set: str, /) -> list[Tuple[str, str]]:
         """
-        Scrape Rebrickable's HTML and return a list of
+        Scrape LEGO's official building-instructions page and return a list
-        (filename_slug, download_url). Duplicate slugs get _1, _2, …
+        of (filename_slug, download_url). Duplicate slugs get _1, _2, …
         """
-        page_url = f"https://rebrickable.com/instructions/{set}/"
+        # LEGO's page wants the bare set number, no "-1" version suffix
+        number, _, _version = set.partition('-')
+
+        page_url = f"https://www.lego.com/en-us/service/building-instructions/{number}"  # noqa: E501
         logger.debug(f"[find_instructions] fetching HTML from {page_url!r}")
 
         # Use plain requests instead of cloudscraper
@@ -295,23 +287,29 @@ class BrickInstructions(object):
             raise ErrorException(f'Failed to load instructions page for {set}. HTTP {resp.status_code}')
 
         soup = BeautifulSoup(resp.content, 'html.parser')
-        # Match download links with or without query parameters (e.g., ?cfe=timestamp&cfk=key)
-        link_re = re.compile(r'^/instructions/\d+/.+/download/')
 
+        # Each booklet is a "bi-card" div. Attribute order on this page is
+        # not stable between requests, so every lookup has to stay scoped
+        # inside its own card rather than collecting all h3s/links
+        # page-wide and pairing them up by position, that can silently
+        # mismatch a label with the wrong link.
         raw: list[tuple[str, str]] = []
-        for a in soup.find_all('a', href=link_re):
+        for card in soup.find_all('div', attrs={'data-test': 'bi-card'}):
-            img = a.find('img', alt=True) # type: ignore
+            label = card.find('h3')
-            if not img or set not in img['alt']: # type: ignore
+            link = card.find('a', attrs={'data-test': 'bi-card-link'})
+
+            if label is None or link is None or not link.get('href'):
                 continue
 
-            # Turn the alt text into a slug
+            # Prefix with the set number: filenames are how a downloaded
-            alt_text = img['alt'].removeprefix('LEGO Building Instructions for ') # type: ignore
+            # instruction later gets matched back to its set (see
-            slug = re.sub(r'[^A-Za-z0-9]+', '-', alt_text).strip('-')
+            # BrickInstructions.__init__), and LEGO's own card labels never
+            # mention the set number, only the product name and booklet.
+            card_slug = re.sub(r'[^A-Za-z0-9]+', '-', label.get_text(strip=True)).strip('-')  # noqa: E501
+            slug = f'{set}-{card_slug}'
+            download_url = urljoin('https://www.lego.com', link['href'])
 
-            # Build the absolute download URL - this preserves query parameters
+            logger.debug(f"[find_instructions] Found download link: {download_url}")  # noqa: E501
-            # BeautifulSoup's a['href'] includes the full href with ?cfe=...&cfk=... params
-            download_url = urljoin('https://rebrickable.com', a['href']) # type: ignore
-            logger.debug(f"[find_instructions] Found download link: {download_url}")
             raw.append((slug, download_url))
 
         if not raw:

bricktracker/version.py

@@ -1,4 +1,4 @@
 from typing import Final
 
-__version__: Final[str] = '1.4.0'
+__version__: Final[str] = '1.4.2'
 __database_version__: Final[int] = 27

bricktracker/views/instructions.py

@@ -147,7 +147,7 @@ def do_upload() -> Response:
     return redirect(url_for('instructions.list'))
 
 
-# Download instructions from Rebrickable
+# Download instructions from LEGO.com
 @instructions_page.route('/download/', methods=['GET'])
 @login_required
 @exception_handler(__file__)
@@ -185,11 +185,11 @@ def do_download() -> str:
     if request.args.get('peeron_loaded'):
         return _render_peeron_select_page(set)
 
-    # Try Rebrickable first
+    # Try LEGO.com first
     try:
         from .instructions import BrickInstructions
         rebrickable_instructions = BrickInstructions.find_instructions(set)
-        # Standard Rebrickable instructions found
+        # Standard LEGO.com instructions found
         return render_template(
             'instructions.html',
             download=True,
@@ -200,7 +200,7 @@ def do_download() -> str:
             messages=MESSAGES
         )
     except Exception:
-        # Rebrickable failed, check if Peeron has instructions (without caching thumbnails yet)
+        # LEGO.com failed, check if Peeron has instructions (without caching thumbnails yet)
         try:
             peeron = PeeronInstructions(set)
             # Just check if pages exist, don't cache thumbnails yet
@@ -223,7 +223,7 @@ def do_download() -> str:
                 download=True,
                 instructions=[],
                 set=set,
-                error='No instructions found on Rebrickable or Peeron',
+                error='No instructions found on LEGO.com or Peeron',
                 path=current_app.config['SOCKET_PATH'],
                 namespace=current_app.config['SOCKET_NAMESPACE'],
                 messages=MESSAGES

templates/peeron_select.html

@@ -10,7 +10,7 @@
         <form method="POST" action="{{ url_for('instructions.do_download') }}">
           <div class="card mb-3">
             <div class="card-header">
-              <h5 class="mb-0"><i class="ri-download-line"></i> Download instructions from Rebrickable</h5>
+              <h5 class="mb-0"><i class="ri-download-line"></i> Download instructions from LEGO.com</h5>
             </div>
             <div class="card-body">
               <div class="mb-3">
@@ -25,7 +25,7 @@
         </form>
         {% if loading_peeron %}
           <div class="alert alert-info" role="alert">
-            <i class="ri-information-line"></i> <strong>Found on Peeron:</strong> {{ set }} was not available on Rebrickable, loading instruction pages from Peeron...
+            <i class="ri-information-line"></i> <strong>Found on Peeron:</strong> {{ set }} was not available on LEGO.com, loading instruction pages from Peeron...
           </div>
 
           <!-- Socket elements for peeron-loader -->
@@ -55,7 +55,7 @@
 
         {% if pages %}
           <div id="peeron-loading-alert" class="alert alert-info" role="alert">
-            <i class="ri-information-line"></i> <strong>Instructions found on Peeron:</strong> {{ set }} was not available on Rebrickable, but {{ pages|length }} instruction pages were found on Peeron.
+            <i class="ri-information-line"></i> <strong>Instructions found on Peeron:</strong> {{ set }} was not available on LEGO.com, but {{ pages|length }} instruction pages were found on Peeron.
             <div id="peeron-cache-progress" class="mt-2 d-none">
               <div class="progress" role="progressbar" aria-label="Caching thumbnails" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100">
                 <div id="peeron-cache-progress-bar" class="progress-bar" style="width: 0%"></div>