From 4ffe3ca24b9d691706d594405b35cc9f295b1861 Mon Sep 17 00:00:00 2001
From: Julie Qiu <julie@golang.org>
Date: Fri, 26 Feb 2021 16:58:48 -0500
Subject: [PATCH] content,internal/frontend: refactor directories section

The following is refactored:

* The UnitPage fields Subdirectories and NestedModules are removed,
  since they are not needed. UnitPage.Directories is used to determine
  if the directories section should be shown.

* The directories section now displays the internal directory at the
  bottom of the page.

For golang/go#44356

Change-Id: I3e6f7e3cacf27e129d6d7ea78d7a89e6e58e4be7
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/296953
Trust: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
---
 .../html/helpers/_unit_directories.tmpl       | 12 +--
 .../static/html/helpers/_unit_outline.tmpl    |  2 +-
 internal/frontend/directory.go                | 85 ++++++++++---------
 internal/frontend/directory_test.go           | 85 +++++++++++--------
 internal/frontend/main.go                     | 14 +--
 5 files changed, 104 insertions(+), 94 deletions(-)

diff --git a/content/static/html/helpers/_unit_directories.tmpl b/content/static/html/helpers/_unit_directories.tmpl
index e79180aef..3047e7b98 100644
--- a/content/static/html/helpers/_unit_directories.tmpl
+++ b/content/static/html/helpers/_unit_directories.tmpl
@@ -12,17 +12,19 @@
     <div class="UnitDirectories-expandButton js-expandAllDirectories">
       <button>Expand all</button>
     </div>
-    {{if .Directories}}
-      <table class="UnitDirectories-table UnitDirectories-table--tree js-expandableTable" data-test-id="UnitDirectories-table">
+      <table class="UnitDirectories-table UnitDirectories-table--tree js-expandableTable"
+             data-test-id="UnitDirectories-table">
         <tr class="UnitDirectories-tableHeader UnitDirectories-tableHeader--tree">
           <th>Path</th>
           <th class="UnitDirectories-desktopSynopsis">Synopsis</th>
         </tr>
-        {{range .Directories}}
-          {{template "directory" .}}
+        {{range $dir := .Directories.External}}
+           {{template "directory" .}}
+        {{end}}
+        {{if .Directories.Internal}}
+           {{template "directory" .Directories.Internal}}
         {{end}}
       </table>
-    {{end}}
   </div>
 {{end}}
 
diff --git a/content/static/html/helpers/_unit_outline.tmpl b/content/static/html/helpers/_unit_outline.tmpl
index 8a47cc7b4..46395adf1 100644
--- a/content/static/html/helpers/_unit_outline.tmpl
+++ b/content/static/html/helpers/_unit_outline.tmpl
@@ -47,7 +47,7 @@
           </a>
         </li>
       {{end}}
-      {{if (or .Subdirectories .NestedModules)}}
+      {{if .Directories}}
         <li role="none">
           <a href="#section-directories" role="treeitem" aria-expanded="false"
               aria-selected="false" aria-level="1" tabindex="-1">
diff --git a/internal/frontend/directory.go b/internal/frontend/directory.go
index f6e5350e7..6ec891473 100644
--- a/internal/frontend/directory.go
+++ b/internal/frontend/directory.go
@@ -13,30 +13,34 @@ import (
 	"golang.org/x/pkgsite/internal/stdlib"
 )
 
-// UnitDirectory is the union of nested modules and subdirectories for a
-// unit organized in a two level tree structure. This content is used in the
+// Directories is the directory listing for all directories in the unit,
+// which is listed in the directories section of the main page.
+type Directories struct {
+	// External contains all of the non-internal directories for the unit.
+	External []*Directory
+
+	// Internal contains the top level internal directory for the unit, if any.
+	Internal *Directory
+}
+
+// Directory is either a nested module or subdirectory of a unit, organized in
+// a two level tree structure. This content is used in the
 // directories section of the unit page.
-type UnitDirectory struct {
+type Directory struct {
 	// Prefix is the prefix of the unit path for the subdirectories.
 	Prefix string
 
 	// Root is the package located at prefix, nil for a directory.
-	Root *Subdirectory
+	Root *DirectoryInfo
 
 	// Subdirectories contains subdirectories with prefix trimmed from their suffix.
-	Subdirectories []*Subdirectory
-}
-
-// NestedModule is a nested module relative to the path of a given unit.
-// This content is used in the Directories section of the unit page.
-type NestedModule struct {
-	Suffix string // suffix after the unit path
-	URL    string
+	Subdirectories []*DirectoryInfo
 }
 
-// Subdirectory is a package in a subdirectory relative to the path of a given
-// unit. This content is used in the Directories section of the unit page.
-type Subdirectory struct {
+// DirectoryInfo contains information about a package or nested module,
+// relative to the path of a given unit. This content is used in the
+// Directories section of the unit page.
+type DirectoryInfo struct {
 	Suffix   string
 	URL      string
 	Synopsis string
@@ -45,24 +49,18 @@ type Subdirectory struct {
 
 // unitDirectories zips the subdirectories and nested modules together in a two
 // level tree hierarchy.
-func unitDirectories(dirs []*Subdirectory, mods []*NestedModule) []*UnitDirectory {
-	var merged []*Subdirectory
-	for _, d := range dirs {
-		merged = append(merged, &Subdirectory{Suffix: d.Suffix,
-			Synopsis: d.Synopsis, URL: d.URL, IsModule: false})
+func unitDirectories(directories []*DirectoryInfo) *Directories {
+	if len(directories) == 0 {
+		return nil
 	}
-	for _, m := range mods {
-		merged = append(merged, &Subdirectory{Suffix: m.Suffix, URL: m.URL, IsModule: true})
-	}
-
 	// Organize the subdirectories into a two level tree hierarchy. The first part of
 	// the unit path suffix for a subdirectory becomes the prefix under which matching
 	// subdirectories are grouped.
-	mappedDirs := make(map[string]*UnitDirectory)
-	for _, d := range merged {
+	mappedDirs := make(map[string]*Directory)
+	for _, d := range directories {
 		prefix := strings.Split(d.Suffix, "/")[0]
 		if _, ok := mappedDirs[prefix]; !ok {
-			mappedDirs[prefix] = &UnitDirectory{Prefix: prefix}
+			mappedDirs[prefix] = &Directory{Prefix: prefix}
 		}
 		d.Suffix = strings.TrimPrefix(d.Suffix, prefix+"/")
 		if prefix == d.Suffix {
@@ -72,15 +70,21 @@ func unitDirectories(dirs []*Subdirectory, mods []*NestedModule) []*UnitDirector
 		}
 	}
 
-	var sorted []*UnitDirectory
-	for _, p := range mappedDirs {
-		sorted = append(sorted, p)
+	section := &Directories{}
+	for prefix, dir := range mappedDirs {
+		if prefix == "internal" {
+			section.Internal = dir
+		} else {
+			section.External = append(section.External, dir)
+		}
 	}
-	sort.Slice(sorted, func(i, j int) bool { return sorted[i].Prefix < sorted[j].Prefix })
-	return sorted
+	sort.Slice(section.External, func(i, j int) bool {
+		return section.External[i].Prefix < section.External[j].Prefix
+	})
+	return section
 }
 
-func getNestedModules(ctx context.Context, ds internal.DataSource, um *internal.UnitMeta, sds []*Subdirectory) ([]*NestedModule, error) {
+func getNestedModules(ctx context.Context, ds internal.DataSource, um *internal.UnitMeta, sds []*DirectoryInfo) ([]*DirectoryInfo, error) {
 	nestedModules, err := ds.GetNestedModules(ctx, um.ModulePath)
 	if err != nil {
 		return nil, err
@@ -91,7 +95,7 @@ func getNestedModules(ctx context.Context, ds internal.DataSource, um *internal.
 	for _, dir := range sds {
 		excludedSuffixes[dir.Suffix] = true
 	}
-	var mods []*NestedModule
+	var mods []*DirectoryInfo
 	for _, m := range nestedModules {
 		if m.SeriesPath() == internal.SeriesPathForModule(um.ModulePath) {
 			continue
@@ -103,16 +107,17 @@ func getNestedModules(ctx context.Context, ds internal.DataSource, um *internal.
 		if excludedSuffixes[suffix] {
 			continue
 		}
-		mods = append(mods, &NestedModule{
-			URL:    constructUnitURL(m.ModulePath, m.ModulePath, internal.LatestVersion),
-			Suffix: suffix,
+		mods = append(mods, &DirectoryInfo{
+			URL:      constructUnitURL(m.ModulePath, m.ModulePath, internal.LatestVersion),
+			Suffix:   suffix,
+			IsModule: true,
 		})
 	}
 	return mods, nil
 }
 
-func getSubdirectories(um *internal.UnitMeta, pkgs []*internal.PackageMeta) []*Subdirectory {
-	var sdirs []*Subdirectory
+func getSubdirectories(um *internal.UnitMeta, pkgs []*internal.PackageMeta) []*DirectoryInfo {
+	var sdirs []*DirectoryInfo
 	for _, pm := range pkgs {
 		if um.Path == pm.Path {
 			continue
@@ -123,7 +128,7 @@ func getSubdirectories(um *internal.UnitMeta, pkgs []*internal.PackageMeta) []*S
 			// list them.
 			continue
 		}
-		sdirs = append(sdirs, &Subdirectory{
+		sdirs = append(sdirs, &DirectoryInfo{
 			URL:      constructUnitURL(pm.Path, um.ModulePath, linkVersion(um.Version, um.ModulePath)),
 			Suffix:   internal.Suffix(pm.Path, um.Path),
 			Synopsis: pm.Synopsis,
diff --git a/internal/frontend/directory_test.go b/internal/frontend/directory_test.go
index 8b671febb..bed4989bf 100644
--- a/internal/frontend/directory_test.go
+++ b/internal/frontend/directory_test.go
@@ -35,12 +35,12 @@ func TestGetNestedModules(t *testing.T) {
 
 	for _, test := range []struct {
 		modulePath     string
-		subdirectories []*Subdirectory
-		want           []*NestedModule
+		subdirectories []*DirectoryInfo
+		want           []*DirectoryInfo
 	}{
 		{
 			modulePath: "cloud.google.com/go",
-			want: []*NestedModule{
+			want: []*DirectoryInfo{
 				{
 					Suffix: "pubsub",
 					URL:    "/cloud.google.com/go/pubsub",
@@ -68,7 +68,7 @@ func TestGetNestedModules(t *testing.T) {
 		},
 		{
 			modulePath: "cloud.google.com/go/storage",
-			want: []*NestedModule{
+			want: []*DirectoryInfo{
 				{
 					Suffix: "module",
 					URL:    "/cloud.google.com/go/storage/module",
@@ -81,7 +81,7 @@ func TestGetNestedModules(t *testing.T) {
 		},
 		{
 			modulePath: "cloud.google.com/go/storage",
-			subdirectories: []*Subdirectory{
+			subdirectories: []*DirectoryInfo{
 				{
 					Suffix: "module",
 					URL:    "/cloud.google.com/go/storage/module",
@@ -94,7 +94,7 @@ func TestGetNestedModules(t *testing.T) {
 		},
 		{
 			modulePath: "cloud.google.com/go/storage/v9",
-			subdirectories: []*Subdirectory{
+			subdirectories: []*DirectoryInfo{
 				{
 					Suffix: "module",
 					URL:    "/cloud.google.com/go/storage/v9/module",
@@ -110,6 +110,9 @@ func TestGetNestedModules(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
+			for _, w := range test.want {
+				w.IsModule = true
+			}
 			if diff := cmp.Diff(test.want, got); diff != "" {
 				t.Errorf("mismatch (-want +got):\n%s", diff)
 			}
@@ -118,53 +121,63 @@ func TestGetNestedModules(t *testing.T) {
 }
 
 func TestUnitDirectories(t *testing.T) {
-	subdirectories := []*Subdirectory{
+	subdirectories := []*DirectoryInfo{
 		{Suffix: "accessapproval"},
+		{Suffix: "accessapproval/internal"},
 		{Suffix: "accessapproval/cgi"},
 		{Suffix: "accessapproval/cookiejar"},
+		{Suffix: "accessapproval/cookiejar/internal"},
 		{Suffix: "fgci"},
 		{Suffix: "httptrace"},
 		{Suffix: "internal/bytesconv"},
 		{Suffix: "internal/json"},
 		{Suffix: "zoltan"},
 	}
-	nestedModules := []*NestedModule{
-		{Suffix: "httptest"},
+	nestedModules := []*DirectoryInfo{
+		{Suffix: "httptest", IsModule: true},
+		{Suffix: "pubsub/internal", IsModule: true},
 	}
-
-	got := unitDirectories(subdirectories, nestedModules)
-	want := []*UnitDirectory{
-		{
-			Prefix: "accessapproval",
-			Root:   &Subdirectory{Suffix: "accessapproval"},
-			Subdirectories: []*Subdirectory{
-				{Suffix: "cgi"},
-				{Suffix: "cookiejar"},
+	got := unitDirectories(append(subdirectories, nestedModules...))
+	want := &Directories{
+		External: []*Directory{
+			{
+				Prefix: "accessapproval",
+				Root:   &DirectoryInfo{Suffix: "accessapproval"},
+				Subdirectories: []*DirectoryInfo{
+					{Suffix: "internal"},
+					{Suffix: "cgi"},
+					{Suffix: "cookiejar"},
+					{Suffix: "cookiejar/internal"},
+				},
+			},
+			{
+				Prefix: "fgci",
+				Root:   &DirectoryInfo{Suffix: "fgci"},
+			},
+			{
+				Prefix: "httptest",
+				Root:   &DirectoryInfo{Suffix: "httptest", IsModule: true},
+			},
+			{
+				Prefix: "httptrace",
+				Root:   &DirectoryInfo{Suffix: "httptrace"},
+			},
+			{
+				Prefix:         "pubsub",
+				Subdirectories: []*DirectoryInfo{{Suffix: "internal", IsModule: true}},
+			},
+			{
+				Prefix: "zoltan",
+				Root:   &DirectoryInfo{Suffix: "zoltan"},
 			},
 		},
-		{
-			Prefix: "fgci",
-			Root:   &Subdirectory{Suffix: "fgci"},
-		},
-		{
-			Prefix: "httptest",
-			Root:   &Subdirectory{Suffix: "httptest", IsModule: true},
-		},
-		{
-			Prefix: "httptrace",
-			Root:   &Subdirectory{Suffix: "httptrace"},
-		},
-		{
+		Internal: &Directory{
 			Prefix: "internal",
-			Subdirectories: []*Subdirectory{
+			Subdirectories: []*DirectoryInfo{
 				{Suffix: "bytesconv"},
 				{Suffix: "json"},
 			},
 		},
-		{
-			Prefix: "zoltan",
-			Root:   &Subdirectory{Suffix: "zoltan"},
-		},
 	}
 
 	if diff := cmp.Diff(want, got); diff != "" {
diff --git a/internal/frontend/main.go b/internal/frontend/main.go
index 03843429d..bc4a1db2d 100644
--- a/internal/frontend/main.go
+++ b/internal/frontend/main.go
@@ -22,16 +22,9 @@ import (
 
 // MainDetails contains data needed to render the unit template.
 type MainDetails struct {
-	// NestedModules are nested modules relative to the path for the unit.
-	NestedModules []*NestedModule
-
-	// Subdirectories are packages in subdirectories relative to the path for
-	// the unit.
-	Subdirectories []*Subdirectory
-
 	// Directories are packages and nested modules relative to the path for the
 	// unit.
-	Directories []*UnitDirectory
+	Directories *Directories
 
 	// Licenses contains license metadata used in the header.
 	Licenses []LicenseMetadata
@@ -203,12 +196,9 @@ func fetchMainDetails(ctx context.Context, ds internal.DataSource, um *internal.
 		return nil, err
 	}
 	isTaggedVersion := versionType != version.TypePseudo
-
 	return &MainDetails{
 		ExpandReadme:      expandReadme,
-		NestedModules:     nestedModules,
-		Subdirectories:    subdirectories,
-		Directories:       unitDirectories(subdirectories, nestedModules),
+		Directories:       unitDirectories(append(subdirectories, nestedModules...)),
 		Licenses:          transformLicenseMetadata(um.Licenses),
 		CommitTime:        absoluteTime(um.CommitTime),
 		Readme:            readme.HTML,