Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 UI updates for standardized explorer names, explore tagging, and an explorer-tiles component #3158

Merged
merged 16 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions adminSiteClient/AdminApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { TestIndexPage } from "./TestIndexPage.js"
import { NotFoundPage } from "./NotFoundPage.js"
import { PostEditorPage } from "./PostEditorPage.js"
import { DeployStatusPage } from "./DeployStatusPage.js"
import { ExplorerTagsPage } from "./ExplorerTagsPage.js"
import { SuggestedChartRevisionApproverPage } from "./SuggestedChartRevisionApproverPage.js"
import { SuggestedChartRevisionListPage } from "./SuggestedChartRevisionListPage.js"
import { SuggestedChartRevisionImportPage } from "./SuggestedChartRevisionImportPage.js"
Expand Down Expand Up @@ -303,6 +304,11 @@ export class AdminApp extends React.Component<{
path="/deploys"
component={DeployStatusPage}
/>
<Route
exact
path="/explorer-tags"
component={ExplorerTagsPage}
/>
<Route
exact
path="/suggested-chart-revisions"
Expand Down
7 changes: 7 additions & 0 deletions adminSiteClient/AdminSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ export const AdminSidebar = (): JSX.Element => (
<Link to="/explorers">
<FontAwesomeIcon icon={faCoffee} /> Explorers
</Link>
<ul>
<li>
<Link to="/explorer-tags">
<FontAwesomeIcon icon={faTag} /> Explorer Tags
</Link>
</li>
</ul>
Comment on lines +47 to +53
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably nothing to change now

this is a bit unexpected - but maybe unavoidable given the git/db limbo. When first encountering this, it gave me the impression that "explorer tags" were different from the regular ones.

Also given how seldom this is going to be used beyond the first tagging effort, it seems like there is a mismatch with the prime real estate this menu item will be permanently using.

Maybe worth considering complementing this with a more contextual link to /explorer-tags on every explorer row?

</li>
<li className="header">DATA</li>

Expand Down
238 changes: 238 additions & 0 deletions adminSiteClient/ExplorerTagsPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
import React from "react"
import { observer } from "mobx-react"
import { action, computed, observable, runInAction } from "mobx"

import { AdminLayout } from "./AdminLayout.js"
import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js"
import { DbChartTagJoin } from "@ourworldindata/utils"
import {
GetAllExplorersRoute,
GetAllExplorersTagsRoute,
} from "../explorer/ExplorerConstants.js"
import { EditableTags } from "./EditableTags.js"
import { ExplorerProgram } from "../explorer/ExplorerProgram.js"
import cx from "classnames"

type ExplorerWithTags = {
slug: string
tags: DbChartTagJoin[]
}

@observer
export class ExplorerTagsPage extends React.Component {
static contextType = AdminAppContext
context!: AdminAppContextType
@observable explorersWithTags: ExplorerWithTags[] = []
@observable explorers: ExplorerProgram[] = []
@observable tags: DbChartTagJoin[] = []
@observable newExplorerSlug = ""
@observable newExplorerTags: DbChartTagJoin[] = []

componentDidMount() {
this.getData()
}
ikesau marked this conversation as resolved.
Show resolved Hide resolved

// Don't show explorers that already have tags
@computed get filteredExplorers() {
return this.explorers.filter((explorer) => {
return !this.explorersWithTags.find((e) => e.slug === explorer.slug)
})
}

render() {
return (
<AdminLayout title="Explorer Tags">
<main className="ExplorerTags">
<h3>Explorer tags</h3>
<p>
This page is for managing the tags for each explorer.
Explorer data is currently stored in{" "}
<a href="https://github.com/owid/owid-content">git</a>,
but tags are stored in the database, which means it's
hard to strictly enforce{" "}
<a href="https://en.wikipedia.org/wiki/Referential_integrity">
referential integrity.
</a>
</p>
<p>
If you update an explorer's slug, you'll need to delete
the old row in this table and create a new one for the
new slug.
</p>

<table className="table table-bordered">
<thead>
<tr>
<th style={{ width: "45%" }}>Explorer</th>
<th style={{ width: "45%" }}>Tags</th>
<th style={{ width: "10%" }}>Controls</th>
</tr>
</thead>
<tbody>
{/* Existing Explorers Rows */}
{this.explorersWithTags.map((explorer) => {
const isSlugValid = this.explorers.find(
(e) => e.slug === explorer.slug
)

return (
<tr
key={explorer.slug}
className={cx({
"table-danger": !isSlugValid,
})}
>
<td>
{explorer.slug}
{isSlugValid ? null : (
<p>
<strong>
❗️ this slug doesn't
match any explorer in
the database - is it for
an explorer that has
been deleted or renamed?
</strong>
</p>
)}
</td>
<td>
<EditableTags
tags={explorer.tags}
suggestions={this.tags}
onSave={(tags) => {
this.saveTags(
explorer.slug,
tags
)
}}
/>
</td>
<td>
<button
className="btn btn-danger"
onClick={() =>
this.deleteExplorerTags(
explorer.slug
)
}
>
Delete
</button>
</td>
</tr>
)
})}
{/* New Explorer Row */}
<tr>
<td>
<select
value={this.newExplorerSlug}
onChange={(e) => {
this.newExplorerSlug =
e.target.value
}}
>
<option value="">
Select an explorer
</option>
{this.filteredExplorers.map(
(explorer) => (
<option
key={explorer.slug}
value={explorer.slug}
>
{explorer.slug}
</option>
)
)}
</select>
</td>
<td>
<EditableTags
// This component clones the `tags` prop and doesn't rerender when that prop changes,
// meaning the tags don't clear when you save a new explorer.
// So we force a rerender by setting a key which updates when an explorer is created
// Unfortunately it also resets if if you delete a row before saving the new explorer
// But that seems like a rare edge case, and I don't want to rewrite the EditableTags component
key={this.explorersWithTags.length}
tags={[]}
suggestions={this.tags}
onSave={(tags) => {
this.newExplorerTags = tags
}}
/>
</td>
<td>
<button
className="btn btn-primary"
disabled={
!this.newExplorerSlug ||
!this.newExplorerTags.length
}
title="Enter a slug and at least one tag to save"
onClick={() => this.saveNewExplorer()}
>
Save
</button>
</td>
</tr>
</tbody>
</table>
</main>
</AdminLayout>
)
}

async getData() {
const [{ tags }, explorersWithTags, explorers] = await Promise.all([
this.context.admin.getJSON("/api/tags.json") as Promise<{
tags: DbChartTagJoin[]
}>,
this.context.admin.getJSON(GetAllExplorersTagsRoute) as Promise<{
explorers: ExplorerWithTags[]
}>,
this.context.admin.getJSON(GetAllExplorersRoute) as Promise<{
explorers: ExplorerProgram[]
}>,
])
runInAction(() => {
this.tags = tags
this.explorersWithTags = explorersWithTags.explorers
this.explorers = explorers.explorers
})
}

async saveTags(slug: string, tags: DbChartTagJoin[]) {
const tagIds = tags.map((tag) => tag.id)
await this.context.admin.requestJSON(
`/api/explorer/${slug}/tags`,
{
tagIds,
},
"POST"
)
}

async deleteExplorerTags(slug: string) {
if (
window.confirm(
`Are you sure you want to delete the tags for "${slug}"?`
)
) {
await this.context.admin.requestJSON(
`/api/explorer/${slug}/tags`,
{},
"DELETE"
)
await this.getData()
}
}

@action.bound async saveNewExplorer() {
await this.saveTags(this.newExplorerSlug, this.newExplorerTags)
await this.getData()
this.newExplorerTags = []
this.newExplorerSlug = ""
}
ikesau marked this conversation as resolved.
Show resolved Hide resolved
}
21 changes: 8 additions & 13 deletions adminSiteServer/adminRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
DefaultNewExplorerSlug,
EXPLORERS_PREVIEW_ROUTE,
GetAllExplorersRoute,
GetAllExplorersTagsRoute,
} from "../explorer/ExplorerConstants.js"
import {
ExplorerProgram,
Expand Down Expand Up @@ -253,6 +254,12 @@ adminRouter.get(`/${GetAllExplorersRoute}`, async (req, res) => {
res.send(await explorerAdminServer.getAllExplorersCommand())
})

adminRouter.get(`/${GetAllExplorersTagsRoute}`, async (_, res) => {
return res.send({
explorers: await db.getExplorerTags(db.knexInstance()),
})
Comment on lines +258 to +260
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not absolutely necessary now, but maybe wrap in read-only transaction?

})

adminRouter.get(`/${EXPLORERS_PREVIEW_ROUTE}/:slug`, async (req, res) => {
const slug = slugify(req.params.slug)
const filename = slug + EXPLORER_FILE_SUFFIX
Expand All @@ -275,16 +282,13 @@ adminRouter.get("/datapage-preview/:id", async (req, res) => {
const variableId = expectInt(req.params.id)
const variableMetadata = await getVariableMetadata(variableId)
if (!variableMetadata) throw new JsonError("No such variable", 404)
const publishedExplorersBySlug =
await explorerAdminServer.getAllPublishedExplorersBySlugCached()

res.send(
await renderDataPageV2({
variableId,
variableMetadata,
isPreviewing: true,
useIndicatorGrapherConfigs: true,
publishedExplorersBySlug,
})
)
})
Expand All @@ -293,16 +297,7 @@ adminRouter.get("/grapher/:slug", async (req, res) => {
const entity = await Chart.getBySlug(req.params.slug)
if (!entity) throw new JsonError("No such chart", 404)

const explorerAdminServer = new ExplorerAdminServer(GIT_CMS_DIR)
const publishedExplorersBySlug =
await explorerAdminServer.getAllPublishedExplorersBySlug()

res.send(
await renderPreviewDataPageOrGrapherPage(
entity.config,
publishedExplorersBySlug
)
)
res.send(await renderPreviewDataPageOrGrapherPage(entity.config))
})

const gitCmsServer = new GitCmsServer({
Expand Down
Loading
Loading