Skip to content

Commit

Permalink
🐝 wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@sophiamersmann
sophiamersmann committed Aug 8, 2024
1 parent 3ecbe66 commit fd9a5a6
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 58 deletions.
44 changes: 31 additions & 13 deletions adminSiteClient/AbstractChartEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ export abstract class AbstractChartEditor<
@observable.ref savedPatchConfig: GrapherInterface = {}
@observable.ref parentConfig: GrapherInterface | undefined = undefined

// TODO: does this work?
completeDefaultConfig = mergeGrapherConfigs(
new Grapher().fullObject,
defaultGrapherConfig
)

constructor(props: { manager: Manager }) {
this.manager = props.manager
this.previewMode =
Expand All @@ -67,22 +73,25 @@ export abstract class AbstractChartEditor<
}

@computed get liveConfig(): GrapherInterface {
return this.grapher.object
return this.grapher.fullObject
}

@computed get fullConfig(): GrapherInterface {
// todo: remove serialised keys that are not part of the schema
return this.liveConfig
}

@computed get patchConfig(): GrapherInterface {
const { liveConfig, parentConfig } = this
if (!parentConfig) return liveConfig
// Grapher's toObject method doesn't include default values,
// but the patch config might need to overwrite non-default values
// from the parent layer. That's why we merge the default grapher config
// in here. The parent config also contains defaults, meaning we're
// getting rid of the defaults when we diff against the parent config below.
const liveConfigWithDefaults = mergeGrapherConfigs(
defaultGrapherConfig,
liveConfig
)
return diffGrapherConfigs(liveConfigWithDefaults, parentConfig)
const { fullConfig, parentConfig } = this
if (!parentConfig) {
return diffGrapherConfigs(fullConfig, this.completeDefaultConfig)
}
return diffGrapherConfigs(fullConfig, parentConfig)
}

@computed get parentConfigWithoutDefaults(): GrapherInterface | undefined {
if (!this.parentConfig) return undefined
return diffGrapherConfigs(this.parentConfig, defaultGrapherConfig)
}

@computed get isModified(): boolean {
Expand All @@ -96,6 +105,15 @@ export abstract class AbstractChartEditor<
return new EditorFeatures(this)
}

// TODO: only works for first level at the moment
isPropertyInherited(property: keyof GrapherInterface): boolean {
if (!this.parentConfigWithoutDefaults) return false
return (
!Object.hasOwn(this.patchConfig, property) &&
Object.hasOwn(this.parentConfigWithoutDefaults, property)
)
}

abstract get isNewGrapher(): boolean
abstract get availableTabs(): EditorTab[]

Expand Down
53 changes: 38 additions & 15 deletions adminSiteClient/ChartEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "@ourworldindata/utils"
import { action, computed, observable, runInAction } from "mobx"
import { BAKED_GRAPHER_URL } from "../settings/clientSettings.js"
import { defaultGrapherConfig } from "@ourworldindata/grapher"
import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher"
import {
AbstractChartEditor,
AbstractChartEditorManager,
Expand Down Expand Up @@ -96,31 +96,51 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {
}

@action.bound async updateParentConfig() {
const currentIndicatorId = this.parentConfig?.dimensions?.[0].variableId
const newIndicatorId = getParentIndicatorIdFromChartConfig(
this.liveConfig
this.fullConfig
)
const currentIndicatorId = this.parentConfig?.dimensions?.[0].variableId
if (newIndicatorId !== currentIndicatorId) {
this.parentConfig = newIndicatorId

// fetch the new parent config if the indicator has changed
let newParentConfig: GrapherInterface | undefined
if (
currentIndicatorId === undefined ||
newIndicatorId !== currentIndicatorId
) {
newParentConfig = newIndicatorId
? await fetchParentConfigForChart(
this.manager.admin,
newIndicatorId
)
: undefined
}

// it's intentional that we don't use this.patchConfig here
const patchConfig = diffGrapherConfigs(
this.liveConfig,
this.parentConfig ?? {}
const currentPatchConfig = this.patchConfig

const completeConfig = mergeGrapherConfigs(
new Grapher().fullObject,
defaultGrapherConfig
)

// first, remove outdated parent values from the current grapher instance
this.grapher.updateFromObject(
mergeGrapherConfigs(completeConfig, currentPatchConfig)
)
const config = mergeGrapherConfigs(this.parentConfig ?? {}, patchConfig)

this.grapher.updateFromObject(config)
// then, merge the new parent values into the current grapher instance
if (newParentConfig) {
this.grapher.updateFromObject(
mergeGrapherConfigs(newParentConfig, currentPatchConfig)
)
}

// this.grapher.updateFromObject(updatedObject)
// TODO(inheritance): what does this do? is this necessary?
this.grapher.updateAuthoredVersion({
...this.grapher.toObject(),
})
this.grapher.updateAuthoredVersion(
mergeGrapherConfigs(newParentConfig ?? {}, currentPatchConfig)
)

this.parentConfig = newParentConfig
}

async saveGrapher({
Expand Down Expand Up @@ -211,7 +231,10 @@ export async function fetchParentConfigForChart(
const indicatorChart = await admin.getJSON(
`/api/variables/mergedGrapherConfig/${indicatorId}.json`
)
return mergeGrapherConfigs(defaultGrapherConfig, indicatorChart)
return mergeGrapherConfigs(
mergeGrapherConfigs(new Grapher().fullObject, defaultGrapherConfig),
indicatorChart
)
}

export function isChartEditorInstance(
Expand Down
8 changes: 7 additions & 1 deletion adminSiteClient/EditorBasicTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ class DimensionSlotView<

this.isSelectingVariables = false

this.updateDimensionsAndRebuildTable(dimensionConfigs)
// TODO: why does this sometimes fail?
try {
this.updateDimensionsAndRebuildTable(dimensionConfigs)
} catch {
console.log("updateDimensionsAndRebuildTable failed")
}
this.updateParentConfig()
}

Expand Down Expand Up @@ -138,6 +143,7 @@ class DimensionSlotView<
}

componentDidMount() {
// TODO: re-enable
// We want to add the reaction only after the grapher is loaded, so we don't update the initial chart (as configured) by accident.
when(
() => this.grapher.isReady,
Expand Down
27 changes: 20 additions & 7 deletions adminSiteClient/EditorDataTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,21 @@ class EntityItem extends React.Component<EntityItemProps> {
}

@observer
export class KeysSection extends React.Component<{ grapher: Grapher }> {
export class KeysSection extends React.Component<{
editor: AbstractChartEditor
}> {
@observable.ref dragKey?: EntityName

@computed get grapher() {
return this.props.editor.grapher
}

@action.bound onAddKey(entityName: EntityName) {
this.props.grapher.selection.selectEntity(entityName)
this.grapher.selection.selectEntity(entityName)
}

@action.bound onDragEnd(result: DropResult) {
const { selection } = this.props.grapher
const { selection } = this.grapher
const { source, destination } = result
if (!destination) return

Expand All @@ -105,12 +111,16 @@ export class KeysSection extends React.Component<{ grapher: Grapher }> {
}

render() {
const { grapher } = this.props
const { grapher } = this
const { selection } = grapher
const { unselectedEntityNames, selectedEntityNames } = selection

return (
<Section name="Data to show">
from parent config:{" "}
{this.props.editor
.isPropertyInherited("selectedEntityNames")
.toString()}
<SelectField
onValue={this.onAddKey}
value="Select data"
Expand Down Expand Up @@ -142,11 +152,14 @@ export class KeysSection extends React.Component<{ grapher: Grapher }> {
key={entityName}
grapher={grapher}
entityName={entityName}
onRemove={() =>
onRemove={() => {
selection.deselectEntity(
entityName
)
}
console.log(
selection.numSelectedEntities
)
}}
/>
</div>
)}
Expand Down Expand Up @@ -278,7 +291,7 @@ export class EditorDataTab<
</label>
</div>
</Section>
<KeysSection grapher={editor.grapher} />
<KeysSection editor={editor} />
{features.canSpecifyMissingDataStrategy && (
<MissingDataSection editor={this.props.editor} />
)}
Expand Down
50 changes: 37 additions & 13 deletions adminSiteClient/EditorHistoryTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@ import { observer } from "mobx-react"
import { ChartEditor, Log } from "./ChartEditor.js"
import { Section, Timeago } from "./Forms.js"
import { computed, action, observable } from "mobx"
import { Json, copyToClipboard } from "@ourworldindata/utils"
import {
Json,
copyToClipboard,
diffGrapherConfigs,

Check warning on line 9 in adminSiteClient/EditorHistoryTab.tsx

View workflow job for this annotation

GitHub Actions / eslint 

'diffGrapherConfigs' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 9 in adminSiteClient/EditorHistoryTab.tsx

View workflow job for this annotation

GitHub Actions / eslint 

'diffGrapherConfigs' is defined but never used. Allowed unused vars must match /^_/u
} from "@ourworldindata/utils"
import YAML from "yaml"
import { notification, Modal } from "antd"
import ReactDiffViewer, { DiffMethod } from "react-diff-viewer-continued"
import { defaultGrapherConfig } from "@ourworldindata/grapher"

Check warning on line 14 in adminSiteClient/EditorHistoryTab.tsx

View workflow job for this annotation

GitHub Actions / eslint 

'defaultGrapherConfig' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 14 in adminSiteClient/EditorHistoryTab.tsx

View workflow job for this annotation

GitHub Actions / eslint 

'defaultGrapherConfig' is defined but never used. Allowed unused vars must match /^_/u

function LogCompareModal({
log,
Expand Down Expand Up @@ -128,14 +133,14 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {

@action.bound copyYamlToClipboard() {
// Use the Clipboard API to copy the config into the users clipboard
const chartConfigObject = {
const patchConfig = {
...this.props.editor.patchConfig,
}
delete chartConfigObject.id
delete chartConfigObject.dimensions
delete chartConfigObject.version
delete chartConfigObject.isPublished
const chartConfigAsYaml = YAML.stringify(chartConfigObject)
delete patchConfig.id
delete patchConfig.dimensions
delete patchConfig.version
delete patchConfig.isPublished
const chartConfigAsYaml = YAML.stringify(patchConfig)
void copyToClipboard(chartConfigAsYaml)
notification["success"]({
message: "Copied YAML to clipboard",
Expand All @@ -146,11 +151,8 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
}

render() {
// Avoid modifying the original JSON object
// Due to mobx memoizing computed values, the JSON can be mutated.
const chartConfigObject = {
...this.props.editor.patchConfig,
}
const { patchConfig, fullConfig, parentConfigWithoutDefaults } =
this.props.editor
return (
<div>
{this.logs.map((log, i) => (
Expand Down Expand Up @@ -178,9 +180,31 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
rows={7}
readOnly
className="form-control"
value={JSON.stringify(chartConfigObject, undefined, 2)}
value={JSON.stringify(patchConfig, undefined, 2)}
/>
</Section>
<Section name="Full Config">
<textarea
rows={7}
readOnly
className="form-control"
value={JSON.stringify(fullConfig, undefined, 2)}
/>
</Section>
{parentConfigWithoutDefaults && (
<Section name="Parent Config">
<textarea
rows={7}
readOnly
className="form-control"
value={JSON.stringify(
parentConfigWithoutDefaults,
undefined,
2
)}
/>
</Section>
)}
</div>
)
}
Expand Down
5 changes: 5 additions & 0 deletions packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ it("does not preserve defaults in the object (except for the schema)", () => {
})
})

it("does preserve all defaults in the full config", () => {
// TODO: write tests
// expect(new Grapher().toFullObject()).toEqual()
})

const unit = "% of children under 5"
const name = "Some display name"
const data = {
Expand Down
Loading

0 comments on commit fd9a5a6

Please sign in to comment.