rewrite generate_parent_navs to work on resource paths

the old code was kind of impossible to understand by reading it, so this
is hopefully considerably clearer

incorporealcms/pages.py

@@ -32,17 +32,18 @@ def display_page(path):
 
     try:
         with app.open_instance_resource(resolved_path, 'r') as entry_file:
-            parent_navs = generate_parent_navs(path)
             mtime = datetime.datetime.fromtimestamp(os.path.getmtime(entry_file.name), get_localzone())
             entry = entry_file.read()
-    except FileNotFoundError:
-        logger.warning("requested path '%s' (resolved path '%s') not found!", path, resolved_path)
-        abort(404)
+    except OSError:
+        logger.error("resolved path '%s' could not be opened!", resolved_path)
+        abort(500)
     else:
         md = init_md()
         content = Markup(md.convert(entry))
         logger.debug("file metadata: %s", md.Meta)
 
+        parent_navs = generate_parent_navs(resolved_path)
+
         return render('base.html', title=get_meta_str(md, 'title'), description=get_meta_str(md, 'description'),
                       image=get_meta_str(md, 'image'), base_url=request.base_url, content=content, navs=parent_navs,
                       mtime=mtime.strftime('%Y-%m-%d %H:%M:%S %Z'))
@@ -135,22 +136,32 @@ def resolve_page_file(path):
 
 
 def generate_parent_navs(path):
-    """Create a series of paths/links to navigate up from the given path."""
-    # derive additional path/location stuff based on path
-    resolved_path = resolve_page_file(path)
-    parent_dir = os.path.dirname(resolved_path)
-    parent_path = '/'.join(path[:-1].split('/')[:-1]) + '/'
-
-    logger.debug("path: '%s'; parent path: '%s'; resolved path: '%s'; parent dir: '%s'",
-                 path, parent_path, resolved_path, parent_dir)
-
-    if path in ('index', '/'):
+    """Create a series of paths/links to navigate up from the given resource path."""
+    if path == 'pages/index.md':
+        # bail and return the title suffix (generally the domain name) as a terminal case
         return [(app.config['TITLE_SUFFIX'], '/')]
     else:
-        with app.open_instance_resource(resolved_path, 'r') as entry_file:
-            entry = entry_file.read()
+        if path.endswith('index.md'):
+            # index case: one dirname for foo/bar/index.md -> foo/bar, one for foo/bar -> foo
+            parent_resource_dir = os.path.dirname(os.path.dirname(path))
+        else:
+            # usual case: foo/buh.md -> foo
+            parent_resource_dir = os.path.dirname(path)
+
+        # generate the request path (i.e. what the link will be) for this path, and
+        # also the resource path of this parent (which is always a dir, so always index.md)
+        request_path = f'/{instance_resource_path_to_request_path(path)}'
+        parent_resource_path = os.path.join(parent_resource_dir, 'index.md')
+
+        logger.debug("resource path: '%s'; request path: '%s'; parent resource path: '%s'", path,
+                     request_path, parent_resource_path)
+
         # for issues regarding parser reuse (see lib.init_md) we reinitialize the parser here
         md = init_md()
+
+        # read the resource
+        with app.open_instance_resource(path, 'r') as entry_file:
+            entry = entry_file.read()
         _ = Markup(md.convert(entry))
-        page_name = " ".join(md.Meta.get('title')) if md.Meta.get('title') else f'/{path}'
-        return generate_parent_navs(parent_path) + [(page_name, f'/{path}')]
+        page_name = " ".join(md.Meta.get('title')) if md.Meta.get('title') else request_path
+        return generate_parent_navs(parent_resource_path) + [(page_name, request_path)]

tests/test_pages.py

@@ -24,32 +24,26 @@ def test_resolve_page_file_other_requests_fine():
 def test_generate_page_navs_index(app):
     """Test that the index page has navs to the root (itself)."""
     with app.app_context():
-        assert generate_parent_navs('/') == [('incorporeal.org', '/')]
-
-
-def test_generate_page_navs_alternate_index(app):
-    """Test that the index page (as a page, not a path) also has navs only to the root (by path)."""
-    with app.app_context():
-        assert generate_parent_navs('index') == [('incorporeal.org', '/')]
+        assert generate_parent_navs('pages/index.md') == [('incorporeal.org', '/')]
 
 
 def test_generate_page_navs_subdir_index(app):
     """Test that dir pages have navs to the root and themselves."""
     with app.app_context():
-        assert generate_parent_navs('subdir/') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/')]
+        assert generate_parent_navs('pages/subdir/index.md') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/')]
 
 
 def test_generate_page_navs_subdir_real_page(app):
     """Test that real pages have navs to the root, their parent, and themselves."""
     with app.app_context():
-        assert generate_parent_navs('subdir/page') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/'),
-                                                       ('Page', '/subdir/page')]
+        assert generate_parent_navs('pages/subdir/page.md') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/'),
+                                                                ('Page', '/subdir/page')]
 
 
 def test_generate_page_navs_subdir_with_title_parsing_real_page(app):
     """Test that title metadata is used in the nav text."""
     with app.app_context():
-        assert generate_parent_navs('subdir-with-title/page') == [
+        assert generate_parent_navs('pages/subdir-with-title/page.md') == [
             ('incorporeal.org', '/'),
             ('SUB!', '/subdir-with-title/'),
             ('/subdir-with-title/page', '/subdir-with-title/page')