Skip to content

refactor: simplify courses #4549

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

Merged
merged 8 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from 6 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
51 changes: 4 additions & 47 deletions apps/web/src/fetcher/request-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ import { dataQuery } from './query'
import { endpoint } from '@/api/endpoint'
import { RequestPageData, UuidRevType, UuidType } from '@/data-types'
import { TaxonomyTermType } from '@/fetcher/graphql-types/operations'
import {
buildCoursePageUrl,
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'

Expand All @@ -55,21 +51,10 @@ export async function requestPage(
// users are not handled in uuid query any more
if (uuid.__typename === UuidType.User) return { kind: 'not-found' }

// temporary redirect course pages to course as a fallback for client side navigation
// that does not go through cf-worker
if (uuid.__typename === 'CoursePage') {
const target = buildCoursePageUrl(
uuid.course.alias,
String(uuid.id),
uuid.title
)
return { kind: 'redirect', target }
}

if (
uuid.__typename === UuidRevType.Article ||
uuid.__typename === UuidRevType.CoursePage ||
uuid.__typename === UuidRevType.Page ||
uuid.__typename === UuidRevType.CoursePage || // TODO: remove at some point
uuid.__typename === UuidRevType.Video ||
uuid.__typename === UuidRevType.Event ||
uuid.__typename === UuidRevType.Applet ||
Expand Down Expand Up @@ -260,27 +245,6 @@ export async function requestPage(
}

if (uuid.__typename === UuidType.Course) {
const pageId = getCoursePageIdFromPath(requestPath)

const pages = (content as unknown as EditorCourseDocument).state.pages
if (!pages || !pages.length) return { kind: 'not-found' }

const coursePageUrls = pages.map((page) =>
buildCoursePageUrl(uuid.alias, page.id, page.title)
)
const pageIndex = Math.max(
pages.findIndex(({ id }) => pageId && id.startsWith(pageId)),
0
)
const page = pages.at(pageIndex)

if (!page) return { kind: 'not-found' }

const fullTitle = page.title ? `${page.title} – ${uuid.title}` : uuid.title
const metaTitle =
fullTitle.length < 75 ? fullTitle : (page.title ?? uuid.title)

const canonicalUrl = pageIndex ? coursePageUrls[pageIndex] : uuid.alias
const metaDescription = sharedMetadata.metaDescription?.length
? sharedMetadata.metaDescription
: uuid.title
Expand All @@ -290,14 +254,7 @@ export async function requestPage(
newsletterPopup: false,
entityData: {
...sharedEntityData,
content: {
...(content as EditorCourseDocument),
serloContext: {
activeCoursePageId: pageId,
courseTitle: uuid.title,
coursePageUrls,
},
} as EditorCourseDocument,
content: content as EditorCourseDocument,
typename: UuidType.Course,
title: uuid.title,
schemaData: {
Expand All @@ -308,9 +265,9 @@ export async function requestPage(
},
metaData: {
...sharedMetadata,
title: metaTitle,
title: uuid.title,
contentType: 'course',
canonicalUrl,
canonicalUrl: uuid.alias,
metaDescription,
},
horizonData,
Expand Down
34 changes: 0 additions & 34 deletions apps/web/src/helper/get-course-id-from-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,3 @@ export function getCoursePageIdFromPath(path: string) {
const hashId = path.split('#')[1]
if (hashId) return hashId
}

export function buildCoursePageUrl(
courseAlias: string,
coursePageId: string,
title: string
) {
const aliasParts = courseAlias.split('/')
if (aliasParts.length !== 4) {
throw new Error('Invalid course alias')
}
const base = aliasParts.slice(0, -1).join('/')
return `${base}/${coursePageId.split('-')[0]}/${toSlug(title)}`
}

// Copied from https://github.com/serlo/api.serlo.org/blob/ce94045b513e59da1ddd191b498fe01f6ff6aa0a/packages/server/src/schema/uuid/abstract-uuid/resolvers.ts#L685-L703
function toSlug(title: string) {
return title
.toLowerCase()
.replace(/ä/g, 'ae')
.replace(/ö/g, 'oe')
.replace(/ü/g, 'ue')
.replace(/ß/g, 'ss')
.replace(/á/g, 'a')
.replace(/é/g, 'e')
.replace(/í/g, 'i')
.replace(/ó/g, 'o')
.replace(/ú/g, 'u')
.replace(/ñ/g, 'n')
.replace(/ /g, '-') // replace spaces with hyphens
.replace(/[^\w-]+/g, '') // remove all non-word chars including _
.replace(/--+/g, '-') // replace multiple hyphens
.replace(/^-+/, '') // trim starting hyphen
.replace(/-+$/, '') // trim end hyphen
}
8 changes: 2 additions & 6 deletions apps/web/src/serlo-editor-integration/serlo-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
type StorageFormat,
} from '@editor/package'
import dynamic from 'next/dynamic'
import { useContext, useRef } from 'react'
import { useRef } from 'react'

import { ArticleAddModal } from './components/article-add-modal/article-add-modal'
import { ExternalRevisionLoader } from './components/external-revision-loader'
Expand All @@ -15,7 +15,6 @@ import { extraSerloPlugins } from './extra-serlo-plugins'
import { extraSerloRenderers } from './extra-serlo-renderers'
import { useAuthentication } from '@/auth/use-authentication'
import { useInstanceData } from '@/contexts/instance-context'
import { RevisionViewContext } from '@/contexts/revision-view-context'
import { isProduction } from '@/helper/is-production'
import type { SetEntityMutationData } from '@/mutations/use-set-entity-mutation/types'

Expand Down Expand Up @@ -68,13 +67,10 @@ export function SerloEditor({
const { lang, licenses } = useInstanceData()
const auth = useAuthentication()

const isRevisionView = useContext(RevisionViewContext)
const isNewEntity = !(initialState as { state?: { id?: string } }).state?.id

return (
<SerloOnlyFeaturesContext.Provider
value={{ isRevisionView, licenses, ArticleAddModal }}
>
<SerloOnlyFeaturesContext.Provider value={{ licenses, ArticleAddModal }}>
<Editor
language={lang === 'de' ? 'de' : 'en'}
editorVariant="serlo-org"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export interface ArticleAddModalProps {
}

interface SerloOnlyFeaturesData {
isRevisionView?: boolean
licenses?: LicenseData[]
ArticleAddModal?: (props: ArticleAddModalProps) => JSX.Element
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const ImageGalleryStaticRenderer = lazy(() =>
)

const CourseStaticRenderer = lazy(() =>
import('@editor/plugins/course/static/static').then((module) => ({
import('@editor/plugins/course/static').then((module) => ({
default: module.CourseStaticRenderer,
}))
)
Expand Down
1 change: 0 additions & 1 deletion packages/editor/src/i18n/strings/de/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const staticStrings = {
},
course: {
title: 'Kurse',
showPages: 'Kursübersicht anzeigen',
pages: 'Kursübersicht',
next: 'Weiter',
back: 'Zurück',
Expand Down
1 change: 0 additions & 1 deletion packages/editor/src/i18n/strings/en/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const staticStrings = {
},
course: {
title: 'Course',
showPages: 'Show course overview',
pages: 'Course overview',
next: 'Next',
back: 'Back',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,66 +8,33 @@ import {
faTrashAlt,
} from '@fortawesome/free-solid-svg-icons'

import { type CourseProps } from '..'
import { CourseNavigationRenderer } from '../renderer/course-navigation'
import { type CourseProps } from '.'
import { CourseNavigationRenderer } from './renderer/course-navigation-renderer'

const toolButtonClassnames = cn(
'serlo-button-edit-secondary serlo-tooltip-trigger mr-1 opacity-0 group-focus-within:opacity-100 group-hover:opacity-100'
)

export function CourseNavigation({
courseNavOpen,
setCourseNavOpen,
export function EditorCourseNavigation({
pages,
activePageIndex,
setActivePageIndex,
}: {
courseNavOpen: boolean
setCourseNavOpen: (open: boolean) => void
pages: CourseProps['state']['pages']
activePageIndex: number
setActivePageIndex: (index: number) => void
}) {
const templateStrings = useEditStrings().templatePlugins

function onRemove() {
function handleRemove(index: number) {
if (window.confirm(templateStrings.course.confirmDelete)) {
pages.remove(activePageIndex)
pages.remove(index)
}
}

return (
<CourseNavigationRenderer
open={courseNavOpen}
onOverviewButtonClick={() => setCourseNavOpen(!courseNavOpen)}
pages={pages.map(({ title, id }, index) => {
const isActive = activePageIndex === index
const pagesData = pages.map(({ title, id }, index) => ({
id: id.value,
title: title.value.trim().length ? title.value : '___',
afterLink: renderPageTools(index),
}))

return {
key: id.value,
element: (
<div className="group">
<button
onClick={() => {
if (isActive) return
window.location.hash = `#${pages[index].id.value}`
setActivePageIndex(index)
}}
className={cn(
'serlo-link text-lg leading-browser',
isActive &&
'font-semibold text-almost-black hover:no-underline'
)}
>
{title.value.trim().length ? title.value : '___'}
</button>{' '}
{renderPageTools(index)}
</div>
),
}
})}
/>
)
return <CourseNavigationRenderer pages={pagesData} />

function renderPageTools(index: number) {
return (
Expand Down Expand Up @@ -97,7 +64,10 @@ export function CourseNavigation({
</button>
) : null}
{pages.length > 1 ? (
<button className={toolButtonClassnames} onClick={onRemove}>
<button
className={toolButtonClassnames}
onClick={() => handleRemove(index)}
>
<EditorTooltip text={templateStrings.course.removeCoursePage} />
<FaIcon icon={faTrashAlt} />
</button>
Expand Down
68 changes: 68 additions & 0 deletions packages/editor/src/plugins/course/editor.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { AddButton } from '@editor/editor-ui'
import { useEditStrings } from '@editor/i18n/edit-strings-provider'
import { EntityTitleInput } from '@editor/plugins/serlo-template-plugins/common/entity-title-input'
import { EditorPluginType } from '@editor/types/editor-plugin-type'
import { useEffect } from 'react'
import { v4 as uuidv4 } from 'uuid'

import { EditorCourseNavigation } from './editor-course-navigation'
import type { CourseProps } from './index.jsx'
import { CoursePagesRenderer } from './renderer/course-pages-renderer'

export function CourseEditor(props: CourseProps) {
const { state } = props
const { pages } = state

const editorStrings = useEditStrings()
const courseStrings = editorStrings.templatePlugins.course

// make sure that is at least one page
useEffect(() => {
if (pages.length) return
createPage()
// only on first load
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [])

function handleAddClick() {
const id = createPage()
setTimeout(() => (window.location.hash = `#${id}`), 30)
}

return (
<>
<EditorCourseNavigation pages={pages} />
<br />
<AddButton onClick={handleAddClick}>
{courseStrings.addCoursePage}
</AddButton>

<CoursePagesRenderer
pages={pages.map((page) => {
return {
id: page.id.value,
title: page.title.value,
titleElement: <EntityTitleInput title={page.title} forceFocus />,
contentElement: page.content.render(),
}
})}
/>
</>
)

function createPage() {
const id = generateNewId()
pages.insert(pages.length, {
id,
title: '',
content: { plugin: EditorPluginType.Rows },
})
return id
}

function generateNewId() {
const newId = uuidv4().slice(0, 5)
if (pages.some(({ id }) => id.value === newId)) return generateNewId()
return newId
}
}
Loading