Repository: dart-lang/pub-dev (dart)

Issue:
Refactor handler for /help/ pages
I think we should keep this simple.

Make a single handler for `/help/<page>` which renders `doc/help-<page>.md`.

Currently, help pages have:
 * A route in `frontend/handlers/routes.dart` (for each help page)
 * A handler in `frontend/handlers/misc.dart` (for each help page)
 * A render function in `frontend/templates/misc.dart` (for each help page)
 * A final rendered markdown string in `frontend/templates/misc.dart` (for each help page)

Perhaps we could just have:
 * A single route for `/help/<page>` in `frontend/handlers/routes.dart`.
 * A single handler in `frontend/handlers/misc.dart` which handles all pages and renders them (given how little rendering there is.

If we set `Cache-Control` to something reasonable, we probably don't even need to cache the rendered files in memory (though it won't hurt).

Pull request description:
- Fixes #8021
- `api.md` is renamed to `help-api.md`
- page title is extracted from the first line of the markdown files (if it is a top-level title), we may consider adjusting it e.g. for `content-moderation.md`

Patch:
diff --git a/app/lib/frontend/handlers/misc.dart b/app/lib/frontend/handlers/misc.dart
index 54f37eb215..ad52b91082 100644
--- a/app/lib/frontend/handlers/misc.dart
+++ b/app/lib/frontend/handlers/misc.dart
@@ -32,35 +32,16 @@ import 'cache_control.dart';
 final _log = Logger('pub.handlers.misc');
 
 /// Handles requests for /help
-Future<shelf.Response> helpPageHandler(shelf.Request request) async {
-  return htmlResponse(renderHelpPage());
-}
-
-/// Handles requests for /help/api
-Future<shelf.Response> helpApiPageHandler(shelf.Request request) async {
-  return htmlResponse(renderHelpApiPage());
-}
-
-/// Handles requests for /help/scoring
-Future<shelf.Response> helpPageScoringHandler(shelf.Request request) async {
-  return htmlResponse(renderHelpScoringPage());
-}
-
-/// Handles requests for /help/content-moderation
-Future<shelf.Response> helpPageContentModerationHandler(
-  shelf.Request request,
-) async {
-  return htmlResponse(renderHelpContentModerationPage());
-}
-
-/// Handles requests for /help/search
-Future<shelf.Response> helpPageSearchHandler(shelf.Request request) async {
-  return htmlResponse(renderHelpSearchPage());
-}
-
-/// Handles requests for /help/publishing
-Future<shelf.Response> helpPagePublishingHandler(shelf.Request request) async {
-  return htmlResponse(renderHelpPublishingPage());
+/// Handles requests for /help/<article>
+Future<shelf.Response> helpPageHandler(
+  shelf.Request request, {
+  String? article,
+}) async {
+  final html = renderHelpPage(article: article);
+  if (html == null) {
+    return notFoundHandler(request);
+  }
+  return htmlResponse(html);
 }
 
 /// Handles requests for /policy
diff --git a/app/lib/frontend/handlers/routes.dart b/app/lib/frontend/handlers/routes.dart
index 9643ae85bb..f619128f6e 100644
--- a/app/lib/frontend/handlers/routes.dart
+++ b/app/lib/frontend/handlers/routes.dart
@@ -263,33 +263,14 @@ class PubSiteService {
   @Route.get('/feed.atom')
   Future<Response> atomFeed(Request request) => atomFeedHandler(request);
 
-  /// Renders the help page
+  /// Renders the main help page
   @Route.get('/help')
-  Future<Response> helpPage(Request request) => helpPageHandler(request);
-
-  /// Renders the help page for API
-  @Route.get('/help/api')
-  Future<Response> helpApiPage(Request request) => helpApiPageHandler(request);
-
-  /// Renders the help page for scoring
-  @Route.get('/help/scoring')
-  Future<Response> helpPageScoring(Request request) =>
-      helpPageScoringHandler(request);
-
-  /// Renders the help page for scoring
-  @Route.get('/help/content-moderation')
-  Future<Response> helpPageContentModeration(Request request) =>
-      helpPageContentModerationHandler(request);
-
-  /// Renders the help page for search
-  @Route.get('/help/search')
-  Future<Response> helpPageSearch(Request request) =>
-      helpPageSearchHandler(request);
-
-  /// Renders the help page for publishing
-  @Route.get('/help/publishing')
-  Future<Response> helpPagePublishing(Request request) =>
-      helpPagePublishingHandler(request);
+  Future<Response> helpPageMain(Request request) => helpPageHandler(request);
+
+  /// Renders a help article page
+  @Route.get('/help/<article>')
+  Future<Response> helpPageArticle(Request request, String article) =>
+      helpPageHandler(request, article: article);
 
   /// Renders the policy page
   @Route.get('/policy')
diff --git a/app/lib/frontend/handlers/routes.g.dart b/app/lib/frontend/handlers/routes.g.dart
index ebca32d0eb..5fd492e183 100644
--- a/app/lib/frontend/handlers/routes.g.dart
+++ b/app/lib/frontend/handlers/routes.g.dart
@@ -236,32 +236,12 @@ Router _$PubSiteServiceRouter(PubSiteService service) {
   router.add(
     'GET',
     r'/help',
-    service.helpPage,
+    service.helpPageMain,
   );
   router.add(
     'GET',
-    r'/help/api',
-    service.helpApiPage,
-  );
-  router.add(
-    'GET',
-    r'/help/scoring',
-    service.helpPageScoring,
-  );
-  router.add(
-    'GET',
-    r'/help/content-moderation',
-    service.helpPageContentModeration,
-  );
-  router.add(
-    'GET',
-    r'/help/search',
-    service.helpPageSearch,
-  );
-  router.add(
-    'GET',
-    r'/help/publishing',
-    service.helpPagePublishing,
+    r'/help/<article>',
+    service.helpPageArticle,
   );
   router.add(
     'GET',
diff --git a/app/lib/frontend/templates/misc.dart b/app/lib/frontend/templates/misc.dart
index b534f25dc2..b57144248c 100644
--- a/app/lib/frontend/templates/misc.dart
+++ b/app/lib/frontend/templates/misc.dart
@@ -16,25 +16,36 @@ import 'views/account/unauthorized.dart';
 import 'views/page/error.dart';
 import 'views/page/standalone.dart';
 
-/// The content of `/doc/api.md`
-final _apiMarkdown = _readDocContent('api.md');
-
 /// The content of `/doc/policy.md`
 final _policyMarkdown = _readDocContent('policy.md');
 
 /// The content of `/doc/security.md`
 final _securityMarkdown = _readDocContent('security.md');
 
-/// The content of `/doc/help.md`
-final _helpMarkdown = _readDocContent('help.md');
-
-/// The content of `/doc/help-*.md`
-final _helpScoringMarkdown = _readDocContent('help-scoring.md');
-final _helpSearchMarkdown = _readDocContent('help-search.md');
-final _helpPublishingMarkdown = _readDocContent('help-publishing.md');
-final _helpContentModerationMarkdown = _readDocContent(
-  'help-content-moderation.md',
-);
+/// Loads help articles and stores them as a map with their
+/// basename as a key, e.g.:
+/// - `help.md` -> (title: 'Help', content: ...)
+/// - `help-search.md` -> (title: 'Search', content: ...)
+late final _helpArticles = () {
+  final docDir = io.Directory(static_files.resolveDocDirPath());
+  final files = docDir
+      .listSync()
+      .whereType<io.File>()
+      .where((f) => f.path.endsWith('.md'));
+  final results = <String, ({String? title, d.Node content})>{};
+  for (final file in files) {
+    final basename = p.basename(file.path);
+    if (basename == 'help.md' || basename.startsWith('help-')) {
+      final content = file.readAsStringSync();
+      final firstLine = content.split('\n').first.trim();
+      final title = firstLine.startsWith('# ')
+          ? firstLine.substring(2).trim().replaceAll('`', '')
+          : null;
+      results[basename] = (title: title, content: _readDocContent(basename));
+    }
+  }
+  return results;
+}();
 
 late final _sideImage = d.Image.decorative(
   src: static_files.staticUrls.packagesSideImage,
@@ -62,81 +73,21 @@ String renderUnauthorizedPage() {
   );
 }
 
-/// Renders the `doc/api.md`.
-String renderHelpApiPage() {
-  return renderLayoutPage(
-    PageType.standalone,
-    standalonePageNode(
-      _apiMarkdown,
-      sideImage: _sideImage,
-    ),
-    title: 'pub.dev API',
-    canonicalUrl: '/help/api',
-  );
-}
-
-/// Renders the `doc/help.md`.
-String renderHelpPage() {
-  return renderLayoutPage(
-    PageType.standalone,
-    standalonePageNode(
-      _helpMarkdown,
-      sideImage: _sideImage,
-    ),
-    title: 'Help | Dart packages',
-    canonicalUrl: '/help',
-  );
-}
-
-/// Renders the `doc/help-scoring.md`.
-String renderHelpScoringPage() {
-  return renderLayoutPage(
-    PageType.standalone,
-    standalonePageNode(
-      _helpScoringMarkdown,
-      sideImage: _sideImage,
-    ),
-    title: 'Scoring | Dart packages',
-    canonicalUrl: '/help/scoring',
-  );
-}
-
-/// Renders the `doc/help-content-moderation.md`.
-String renderHelpContentModerationPage() {
-  return renderLayoutPage(
-    PageType.standalone,
-    standalonePageNode(
-      _helpContentModerationMarkdown,
-      sideImage: _sideImage,
-    ),
-    title: 'Content Moderation | Pub site',
-    canonicalUrl: '/help/content-moderation',
-  );
-}
-
-/// Renders the `doc/help-search.md`.
-String renderHelpSearchPage() {
-  return renderLayoutPage(
-    PageType.standalone,
-    standalonePageNode(
-      _helpSearchMarkdown,
-      sideImage: _sideImage,
-    ),
-    title: 'Search | Dart packages',
-    canonicalUrl: '/help/search',
-  );
-}
-
-/// Renders the `doc/help-publishing.md`.
-String renderHelpPublishingPage() {
+/// Renders the `doc/help[<-article>].md`.
+String? renderHelpPage({String? article}) {
+  final basename = article == null ? 'help.md' : 'help-$article.md';
+  final page = _helpArticles[basename];
+  if (page == null) {
+    return null;
+  }
   return renderLayoutPage(
     PageType.standalone,
     standalonePageNode(
-      _helpPublishingMarkdown,
+      page.content,
       sideImage: _sideImage,
     ),
-    title: 'Publishing | Dart packages',
-    canonicalUrl: '/help/publishing',
+    title: [page.title, 'Help', 'Dart packages'].nonNulls.toSet().join(' | '),
+    canonicalUrl: article == null ? '/help' : '/help/$article',
   );
 }
 
diff --git a/doc/api.md b/doc/help-api.md
similarity index 100%
rename from doc/api.md
rename to doc/help-api.md


Test patch:
diff --git a/app/test/frontend/templates_test.dart b/app/test/frontend/templates_test.dart
index cbe6da9369..786bfa3462 100644
--- a/app/test/frontend/templates_test.dart
+++ b/app/test/frontend/templates_test.dart
@@ -786,7 +786,7 @@ void main() {
 
     testWithProfile('help page', fn: () async {
       final html = renderHelpPage();
-      expectGoldenFile(html, 'help_page.html');
+      expectGoldenFile(html!, 'help_page.html');
     });
 
     testWithProfile('topics page', fn: () async {