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

refactor: adjust Frontend to new Editor storage format #4465

Draft
wants to merge 19 commits into
base: staging
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
78ed69b
refactor(editor): adjust local storage logic to new storage format
hejtful Feb 7, 2025
66eb06c
refactor(web): adjust converting editor response to state
hejtful Feb 7, 2025
62c19ab
refactor: use Editor onChange in web + some Editor optimizations
hejtful Feb 7, 2025
1136bdd
chore: remove deprecated selectRootDocument
hejtful Feb 7, 2025
d482947
fix(web): type and format after switching to Editor storage format
hejtful Feb 7, 2025
e1b9ff2
refactor(editor): Editor component slightly more readable
hejtful Feb 7, 2025
1c9543b
fix(editor-integration): add missing plugins
hejtful Feb 10, 2025
8ea391f
fix(request-page): wrap Video and Applet content in Rows plugin
hejtful Feb 10, 2025
aa7630a
chore(entity): temporarily wrap entity content in Editor metadata
hejtful Feb 10, 2025
4bdd6bb
merge staging
hejtful Feb 10, 2025
7354088
merge staging 2
hejtful Feb 10, 2025
d3c0318
merge staging 3
hejtful Feb 11, 2025
8f5affa
chore(web): remove temp editor metadata wrappers
hejtful Feb 11, 2025
7796ec6
fix(web): unwrap editor document for serlo renderers
hejtful Feb 11, 2025
8d1029a
refactor(web): add StorageFormat unwrap to parseDocumentString
elbotho Feb 13, 2025
dcd65dc
Merge pull request #4490 from serlo/refactor/cleanup-editor-state-unwrap
hejtful Feb 13, 2025
25bad25
chore(web): don't use raw string
elbotho Feb 13, 2025
7ab902e
fix(web): only render save button if editor state loaded and valid
hejtful Feb 14, 2025
70491c8
fix(web): convert Editor state to API data before saving
hejtful Feb 14, 2025
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
20 changes: 14 additions & 6 deletions apps/web/src/fetcher/create-exercises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import type {
} from '@editor/package'

import { MainUuidType } from './query-types'
import { UuidType } from '@/data-types'
import { parseDocumentString } from '@/helper/parse-document-string'
import { unwrapEditorContent } from '@/serlo-editor-integration/convert-editor-response-to-state'

type BareExercise = Omit<
Extract<MainUuidType, { __typename: 'Exercise' }>,
Expand All @@ -16,10 +18,13 @@ export function createExercise(
): EditorExerciseDocument | undefined {
if (!uuid.currentRevision?.content) return undefined

const { templateContent } = unwrapEditorContent(
UuidType.Exercise,
uuid.currentRevision.content
)

const exercise = {
...(parseDocumentString(
uuid.currentRevision.content
) as EditorExerciseDocument),
...(parseDocumentString(templateContent) as EditorExerciseDocument),
serloContext: {
uuid: uuid.id,
revisionId: uuid.currentRevision.id,
Expand All @@ -41,10 +46,13 @@ export function createExerciseGroup(
): EditorExerciseGroupDocument | undefined {
if (!uuid.currentRevision?.content) return undefined

const { templateContent } = unwrapEditorContent(
UuidType.ExerciseGroup,
uuid.currentRevision.content
)

return {
...(parseDocumentString(
uuid.currentRevision.content
) as EditorExerciseGroupDocument),
...(parseDocumentString(templateContent) as EditorExerciseGroupDocument),
serloContext: {
uuid: uuid.id,
trashed: uuid.trashed,
Expand Down
17 changes: 14 additions & 3 deletions apps/web/src/fetcher/create-taxonomy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '@/data-types'
import { parseDocumentString } from '@/helper/parse-document-string'
import { hasSpecialUrlChars } from '@/helper/urls/check-special-url-chars'
import { unwrapEditorContent } from '@/serlo-editor-integration/convert-editor-response-to-state'

type TaxonomyTerm = Extract<
MainUuidQuery['uuid'],
Expand All @@ -24,10 +25,20 @@ type TaxonomyTermChildrenLevel2 = Extract<
>['children']['nodes'][0]

export function buildTaxonomyData(uuid: TaxonomyTerm): TaxonomyData {
const children = uuid.children.nodes.filter(isActive)
const { templateContent: description } = unwrapEditorContent(
UuidType.TaxonomyTerm,
uuid.description ?? ''
)
const children = uuid.children.nodes.filter(isActive).map((child) => {
const { templateContent: childDescription } = unwrapEditorContent(
UuidType.TaxonomyTerm,
(child as TaxonomyTerm).description ?? ''
)
return { ...child, description: childDescription }
})
return {
description: uuid.description
? (parseDocumentString(uuid.description) as EditorRowsDocument)
description: description
? (parseDocumentString(description) as EditorRowsDocument)
: undefined,
title: uuid.name,
id: uuid.id,
Expand Down
14 changes: 12 additions & 2 deletions apps/web/src/fetcher/request-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
getCoursePageIdFromPath,
} from '@/helper/get-course-id-from-path'
import { parseDocumentString } from '@/helper/parse-document-string'
import { unwrapEditorContent } from '@/serlo-editor-integration/convert-editor-response-to-state'

// ALWAYS start requestPath with slash
export async function requestPage(
Expand Down Expand Up @@ -168,9 +169,14 @@ export async function requestPage(
}
}

const { editorMetadata, templateContent } = unwrapEditorContent(
uuid.__typename,
uuid.currentRevision?.content
)

const content = (await prettifyLinksInState(
uuid.currentRevision?.content
? parseDocumentString(uuid.currentRevision?.content)
? parseDocumentString(templateContent)
: undefined
)) as EditorRowsDocument | EditorCourseDocument

Expand Down Expand Up @@ -205,7 +211,10 @@ export async function requestPage(
trashed: uuid.trashed,
title: uuid.title,
licenseId,
content,
content: {
...editorMetadata,
document: content,
},
isUnrevised: !uuid.currentRevision,
unrevisedRevisions: uuid.revisions?.totalCount,
}
Expand Down Expand Up @@ -316,6 +325,7 @@ export async function requestPage(
newsletterPopup: true,
entityData: {
...sharedEntityData,
content,
typename: UuidType.Page,
},
metaData: {
Expand Down
8 changes: 7 additions & 1 deletion apps/web/src/pages/api/frontend/injection-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import { gql } from 'graphql-request'
import type { NextApiRequest, NextApiResponse } from 'next'

import { endpoint } from '@/api/endpoint'
import { UuidType } from '@/data-types'
import { InjectionOnlyContentQuery } from '@/fetcher/graphql-types/operations'
import { isProduction } from '@/helper/is-production'
import { parseDocumentString } from '@/helper/parse-document-string'
import { unwrapEditorContent } from '@/serlo-editor-integration/convert-editor-response-to-state'

/**
* Allows frontend (and later other) instances to get content of injected entity
Expand Down Expand Up @@ -71,8 +73,12 @@ export default async function handler(
}

if (uuid.__typename === 'ExerciseGroup') {
const content = parseDocumentString(
const { templateContent } = unwrapEditorContent(
UuidType.ExerciseGroup,
uuid.currentRevision.content
)
const content = parseDocumentString(
templateContent
) as EditorExerciseGroupDocument

// use id in hash to load one exercise out of the group
Expand Down
7 changes: 5 additions & 2 deletions apps/web/src/pages/entity/create/[type]/[taxonomyId].tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TemplatePluginType } from '@editor/package'
import { createEmptyDocument, TemplatePluginType } from '@editor/package'
import request, { gql } from 'graphql-request'
import { GetStaticPaths, GetStaticProps } from 'next'

Expand Down Expand Up @@ -45,7 +45,10 @@ function Content({
props: EntityCreateProps
}) {
const initialState = {
plugin: AllowedPlugins[entityType],
...createEmptyDocument('serlo-org'),
document: {
plugin: AllowedPlugins[entityType],
},
Comment on lines +48 to +51

Choose a reason for hiding this comment

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

Maybe createEmptyDocument could generate different initial editor states in the future. Example: createEmptyDocument(editorVariant: 'serlo-org', type: 'article')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't mind, could you please explain the benefits of that in more detail? Is it to avoid this destructuring approach, or are there other benefits?

Choose a reason for hiding this comment

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

When I created this function in the past there was only one root plugin type-generic-content so it made sense to hard code it. Now, there are different types. I think I would slightly prefer having all that logic in function createEmptyDocument. But the way it is now is also fine for me. ☺️

Copy link
Contributor Author

@hejtful hejtful Feb 13, 2025

Choose a reason for hiding this comment

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

Fair point! And it should stay like that, actually. I only exported the createEmptyDocument function through the package as a special case for Serlo (which is also documented by comments in the package index file, and also why it's not mentioned in the README). So it's a temporary hacky solution, and I wouldn't change the original function for it.

It's definitely not great, but I decided that these refactorings need to go step by step, if I tried to make everything perfect on the first go, I'd get lost in it 😁

Choose a reason for hiding this comment

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

Ah, alright. Thanks for clarifying. Going step by step sounds great to me. Just wanted to share my thoughts.

}

const { id: taxonomyParentId } = taxonomyTerm
Expand Down
7 changes: 5 additions & 2 deletions apps/web/src/pages/page/create.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TemplatePluginType } from '@editor/package'
import { createEmptyDocument, TemplatePluginType } from '@editor/package'

import { FrontendClientBase } from '@/components/frontend-client-base/frontend-client-base'
import { Guard } from '@/components/guard'
Expand All @@ -14,7 +14,10 @@ import { renderedPageNoHooks } from '@/helper/rendered-page'
export default renderedPageNoHooks(() => {
const addRevisionProps = {
initialState: {
plugin: TemplatePluginType.Page,
...createEmptyDocument('serlo-org'),
document: {
plugin: TemplatePluginType.Page,
},
},
type: UuidType.Page,
errorType: 'none',
Expand Down
12 changes: 8 additions & 4 deletions apps/web/src/pages/taxonomy/term/create/[parent_id]/[id].tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createEmptyDocument } from '@editor/package'
import { GetServerSideProps } from 'next'

import { FrontendClientBase } from '@/components/frontend-client-base/frontend-client-base'
Expand All @@ -24,10 +25,13 @@ export default renderedPageNoHooks<TaxonomyTermCreateProps>(({ parent }) => {
type={UuidType.TaxonomyTerm}
id={parent}
initialState={{
plugin: 'type-taxonomy',
state: {
term: { name: '' },
description: '{"plugin":"rows"}',
...createEmptyDocument('serlo-org'),
document: {
plugin: 'type-taxonomy',
state: {
term: { name: '' },
description: '{"plugin":"rows"}',
},
},
}}
errorType="none"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export function ExternalRevisionLoader<T>({
: uuid.id

handleReplace({
...(converted.state as T),
...((converted.document || {}).state as T),
revision: 0,
id: 0,
meta_title: '',
Expand Down
10 changes: 5 additions & 5 deletions apps/web/src/serlo-editor-integration/components/save-button.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { AnyEditorDocument } from '@editor/package'
import type { StorageFormat } from '@editor/package'
import { faSave } from '@fortawesome/free-solid-svg-icons'
import { useState } from 'react'
import { type MutableRefObject, useState } from 'react'
import { createPortal } from 'react-dom'

import { SaveModal } from './save-modal'
Expand All @@ -13,12 +13,12 @@ import { useLeaveConfirm } from '@/helper/use-leave-confirm'
export function SaveButton({
onSave,
isChanged,
selectRootDocument,
editorState,
isInTestArea,
}: {
onSave: SerloEditorProps['onSave']
isChanged: boolean
selectRootDocument: () => AnyEditorDocument
editorState: MutableRefObject<StorageFormat>
isInTestArea?: boolean
}) {
const [saveModalOpen, setSaveModalOpen] = useState(false)
Expand All @@ -45,7 +45,7 @@ export function SaveButton({
open={saveModalOpen}
setOpen={setSaveModalOpen}
onSave={onSave}
selectRootDocument={selectRootDocument}
editorState={editorState}
isInTestArea={isInTestArea}
/>
</div>,
Expand Down
26 changes: 13 additions & 13 deletions apps/web/src/serlo-editor-integration/components/save-modal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TemplatePluginType, type AnyEditorDocument } from '@editor/package'
import { type StorageFormat, TemplatePluginType } from '@editor/package'
import { faExclamationCircle } from '@fortawesome/free-solid-svg-icons'
import { useEffect, useState } from 'react'
import { type MutableRefObject, useEffect, useState } from 'react'

import type { SerloEditorProps } from '../serlo-editor'
import { useHandleSave } from '../use-handle-save'
Expand All @@ -17,25 +17,25 @@ export function SaveModal({
open,
setOpen,
onSave,
selectRootDocument,
editorState,
isInTestArea,
}: {
open: boolean
setOpen: (arg0: boolean) => void
onSave: SerloEditorProps['onSave']
selectRootDocument: () => AnyEditorDocument
editorState: MutableRefObject<StorageFormat>
isInTestArea?: boolean
}) {
const serializedRoot = selectRootDocument()
const serializedRootState =
serializedRoot?.state as SupportedTypesSerializedState
const editorDocument = editorState.current.document
const editorDocumentState =
editorDocument?.state as SupportedTypesSerializedState

const licenseId = serializedRootState.licenseId
const changes = serializedRootState.changes
const licenseId = editorDocumentState.licenseId
const changes = editorDocumentState.changes

const { handleSave, pending, hasError } = useHandleSave(
open,
serializedRootState,
editorDocumentState,
onSave
)
const [hasAgreedLicense, setHasAgreedLicense] = useState(false)
Expand All @@ -46,18 +46,18 @@ export function SaveModal({

const licenseAccepted = !licenseId || hasAgreedLicense
const changesFilled = !changes || changesText
const isNoEntity = serializedRoot
const isNoEntity = editorDocument
? [
TemplatePluginType.User,
TemplatePluginType.Page,
TemplatePluginType.Taxonomy,
].includes(serializedRoot.plugin as TemplatePluginType)
].includes(editorDocument.plugin as TemplatePluginType)
: false
const maySave = isNoEntity || (licenseAccepted && changesFilled)
const needsNoReview = isInTestArea || isNoEntity
const isOnlyText = isNoEntity || (needsNoReview && !licenseId && !changes)

const showChanges = serializedRoot ? !isNoEntity : true
const showChanges = editorDocument ? !isNoEntity : true

useEffect(() => {
if (!fireSave) return
Expand Down
Loading