From f6f9bbd2d5a021ed8041b429ae4c23eb4adfc4f5 Mon Sep 17 00:00:00 2001 From: Eric Teubert Date: Sun, 28 Jan 2024 21:42:06 +0100 Subject: [PATCH 1/4] first spike, not happy with where it is going --- client/src/sagas/mediafiles.sagas.ts | 45 ++++++++++++++++++++++++++++ client/src/store/mediafiles.store.ts | 3 ++ js/src/admin/episode.js | 35 ---------------------- lib/ajax/ajax.php | 8 ----- 4 files changed, 48 insertions(+), 43 deletions(-) diff --git a/client/src/sagas/mediafiles.sagas.ts b/client/src/sagas/mediafiles.sagas.ts index b54abb159..9194e4cb0 100644 --- a/client/src/sagas/mediafiles.sagas.ts +++ b/client/src/sagas/mediafiles.sagas.ts @@ -29,6 +29,7 @@ function* initialize(api: PodloveApiClient) { yield takeEvery(mediafiles.DISABLE, handleDisable, api) yield takeEvery(mediafiles.VERIFY, handleVerify, api) yield takeEvery(episode.SAVED, maybeReverify, api) + yield takeEvery(episode.SAVED, maybeUpdateSlug, api) yield throttle( 2000, [mediafiles.ENABLE, mediafiles.DISABLE, mediafiles.UPDATE], @@ -101,6 +102,50 @@ function* maybeReverify(api: PodloveApiClient, action: { type: string; payload: yield all(mediaFiles.map((file) => call(verifyEpisodeAsset, api, episodeId, file.asset_id))) } +// THOUGHTS +// - auto gen logic is both here and in the wordpress saga. needs to at least be +// dry +// - previous slug generation was on PHP side, using WordPress' sanitize_title, +// which I do not want to reimplement in PHP +// - maybe have a dedicated endpoint to push an unsanitized slug? But then I +// might as well send a "just generate me a slug" ping instead and do +// everything on the backend. Sounds best actually. +function* maybeUpdateSlug(api: PodloveApiClient, action: { type: string; payload: object }) { + const changedKeys = Object.keys(action.payload) + const interestingKeys = ['title', 'number'] + const slugNeedsUpdating = changedKeys.some((key) => interestingKeys.includes(key)) + + const generateTitle: boolean = yield select(selectors.settings.autoGenerateEpisodeTitle) + const template: string = yield select(selectors.settings.blogTitleTemplate) + const title: string = yield select(selectors.episode.title) + const episodeNumber: string = yield select(selectors.episode.number) + const mnemonic: string = yield select(selectors.podcast.mnemonic) + // TODO: get from episode? + const seasonNumber: string = '' + const padding: number = yield select(selectors.settings.episodeNumberPadding) + + // So ugly: I don't want to have to know about the title template here, or if it's even used + if (slugNeedsUpdating) { + let newSlug = '' + + if (generateTitle && template) { + const generatedTitle = template + .replace('%mnemonic%', mnemonic || '') + .replace('%episode_number%', (episodeNumber || '').padStart(padding || 0, '0')) + .replace('%season_number%', seasonNumber || '') + .replace('%episode_title%', title || '') + newSlug = generatedTitle + } else { + newSlug = title + } + + // FIXME: `newSlug` is currently just the title. Needs to be slugged. But + // maybe I don't do all that here anyway, see thoughts above. + + yield put(episode.update({ prop: 'slug', value: newSlug })) + } +} + function* verifyEpisodeAsset(api: PodloveApiClient, episodeId: number, assetId: number) { const mediaFiles: MediaFile[] = yield select(selectors.mediafiles.files) const prevMediaFile: MediaFile | undefined = mediaFiles.find((mf) => mf.asset_id == assetId) diff --git a/client/src/store/mediafiles.store.ts b/client/src/store/mediafiles.store.ts index 100559f7e..77abd5cbf 100644 --- a/client/src/store/mediafiles.store.ts +++ b/client/src/store/mediafiles.store.ts @@ -11,11 +11,13 @@ export type MediaFile = { export type State = { is_initializing: boolean + slug_autogeneration_enabled: boolean files: MediaFile[] } export const initialState: State = { is_initializing: true, + slug_autogeneration_enabled: true, files: [], } @@ -86,5 +88,6 @@ export const reducer = handleActions( export const selectors = { isInitializing: (state: State) => state.is_initializing, + slugAutogenerationEnabled: (state: State) => state.slug_autogeneration_enabled, files: (state: State) => state.files, } diff --git a/js/src/admin/episode.js b/js/src/admin/episode.js index 0806d95b2..3959069ae 100644 --- a/js/src/admin/episode.js +++ b/js/src/admin/episode.js @@ -7,32 +7,6 @@ var PODLOVE = PODLOVE || {} PODLOVE.Episode = function (container) { var o = {} - // private - - function maybe_update_episode_slug(title) { - if (o.slug_field.data('auto-update')) { - update_episode_slug(title) - } - } - - // current ajax object to ensure only the latest one is active - var update_episode_slug_xhr - - function update_episode_slug(title) { - if (update_episode_slug_xhr) update_episode_slug_xhr.abort() - - update_episode_slug_xhr = $.ajax({ - url: ajaxurl, - data: { - action: 'podlove-episode-slug', - title: title, - }, - context: o.slug_field, - }).done(function (slug) { - $(this).val(slug).blur() - }) - } - o.slug_field = container.find('[name*=slug]') $('#_podlove_meta_subtitle').count_characters({ @@ -63,12 +37,6 @@ var PODLOVE = PODLOVE || {} } }) - o.slug_field - .data('auto-update', !Boolean(o.slug_field.val())) // only auto-update if it is empty - .on('keyup', function () { - o.slug_field.data('auto-update', false) // stop autoupdate on manual change - }) - var typewatch = (function () { var timer = 0 return function (callback, ms) { @@ -97,9 +65,6 @@ var PODLOVE = PODLOVE || {} // update episode title $('#_podlove_meta_title').attr('placeholder', title) - - // maybe update episode slug - maybe_update_episode_slug(title) }) .trigger('titleHasChanged') diff --git a/lib/ajax/ajax.php b/lib/ajax/ajax.php index 42f218df3..0ce5f72a9 100644 --- a/lib/ajax/ajax.php +++ b/lib/ajax/ajax.php @@ -39,7 +39,6 @@ public function __construct() 'analytics-global-total-downloads', 'analytics-global-total-downloads-by-show', 'analytics-csv-episodes-table', - 'episode-slug', 'admin-news', 'job-create', 'job-get', @@ -915,13 +914,6 @@ public function get_license_parameters_from_url() self::respond_with_json(\Podlove\Model\License::get_license_from_url($_REQUEST['url'])); } - public function episode_slug() - { - echo sanitize_title($_REQUEST['title']); - - exit; - } - private static function analytics_date_condition() { $from = filter_input(INPUT_GET, 'date_from'); From fb2cf37a319117dfcd161242634d89599dadaee7 Mon Sep 17 00:00:00 2001 From: Eric Teubert Date: Fri, 2 Feb 2024 16:16:33 +0100 Subject: [PATCH 2/4] better --- client/src/sagas/mediafiles.sagas.ts | 61 ++++++++++++---------------- client/src/sagas/wordpress.sagas.ts | 8 +++- includes/api/episodes.php | 37 +++++++++++++++++ 3 files changed, 70 insertions(+), 36 deletions(-) diff --git a/client/src/sagas/mediafiles.sagas.ts b/client/src/sagas/mediafiles.sagas.ts index 9194e4cb0..9ae74ad1d 100644 --- a/client/src/sagas/mediafiles.sagas.ts +++ b/client/src/sagas/mediafiles.sagas.ts @@ -5,7 +5,7 @@ import * as mediafiles from '@store/mediafiles.store' import * as episode from '@store/episode.store' import * as wordpress from '@store/wordpress.store' import { MediaFile } from '@store/mediafiles.store' -import { takeFirst } from './helper' +import { takeFirst, channel } from './helper' import { createApi } from './api' import { Action } from 'redux' import { get } from 'lodash' @@ -30,6 +30,8 @@ function* initialize(api: PodloveApiClient) { yield takeEvery(mediafiles.VERIFY, handleVerify, api) yield takeEvery(episode.SAVED, maybeReverify, api) yield takeEvery(episode.SAVED, maybeUpdateSlug, api) + yield takeEvery(wordpress.UPDATE, maybeUpdateSlugFromWP, api) + yield throttle( 2000, [mediafiles.ENABLE, mediafiles.DISABLE, mediafiles.UPDATE], @@ -102,47 +104,36 @@ function* maybeReverify(api: PodloveApiClient, action: { type: string; payload: yield all(mediaFiles.map((file) => call(verifyEpisodeAsset, api, episodeId, file.asset_id))) } -// THOUGHTS -// - auto gen logic is both here and in the wordpress saga. needs to at least be -// dry -// - previous slug generation was on PHP side, using WordPress' sanitize_title, -// which I do not want to reimplement in PHP -// - maybe have a dedicated endpoint to push an unsanitized slug? But then I -// might as well send a "just generate me a slug" ping instead and do -// everything on the backend. Sounds best actually. function* maybeUpdateSlug(api: PodloveApiClient, action: { type: string; payload: object }) { const changedKeys = Object.keys(action.payload) + const interestingKeys = ['title', 'number'] const slugNeedsUpdating = changedKeys.some((key) => interestingKeys.includes(key)) - const generateTitle: boolean = yield select(selectors.settings.autoGenerateEpisodeTitle) - const template: string = yield select(selectors.settings.blogTitleTemplate) - const title: string = yield select(selectors.episode.title) - const episodeNumber: string = yield select(selectors.episode.number) - const mnemonic: string = yield select(selectors.podcast.mnemonic) - // TODO: get from episode? - const seasonNumber: string = '' - const padding: number = yield select(selectors.settings.episodeNumberPadding) + const episodeId: boolean = yield select(selectors.episode.id) + const postTitle: string = yield select(selectors.post.title) - // So ugly: I don't want to have to know about the title template here, or if it's even used if (slugNeedsUpdating) { - let newSlug = '' - - if (generateTitle && template) { - const generatedTitle = template - .replace('%mnemonic%', mnemonic || '') - .replace('%episode_number%', (episodeNumber || '').padStart(padding || 0, '0')) - .replace('%season_number%', seasonNumber || '') - .replace('%episode_title%', title || '') - newSlug = generatedTitle - } else { - newSlug = title - } - - // FIXME: `newSlug` is currently just the title. Needs to be slugged. But - // maybe I don't do all that here anyway, see thoughts above. - - yield put(episode.update({ prop: 'slug', value: newSlug })) + const { result } = yield api.get(`episodes/${episodeId}/build_slug`, { + query: { title: postTitle }, + }) + yield put(episode.update({ prop: 'slug', value: result.slug })) + } +} + +function* maybeUpdateSlugFromWP( + api: PodloveApiClient, + action: { type: string; payload: { prop: string; value: any } } +) { + const episodeId: boolean = yield select(selectors.episode.id) + + if (action.payload.prop == 'title') { + const newTitle = action.payload.value + + const { result } = yield api.get(`episodes/${episodeId}/build_slug`, { + query: { title: newTitle }, + }) + yield put(episode.update({ prop: 'slug', value: result.slug })) } } diff --git a/client/src/sagas/wordpress.sagas.ts b/client/src/sagas/wordpress.sagas.ts index ba3ec7211..ecc4495ad 100644 --- a/client/src/sagas/wordpress.sagas.ts +++ b/client/src/sagas/wordpress.sagas.ts @@ -68,11 +68,17 @@ function* updatePostTitle() { const seasonNumber: string = '' const padding: number = yield select(selectors.settings.episodeNumberPadding) - wordpress.postTitleInput.value = template + const newTitle = template .replace('%mnemonic%', mnemonic || '') .replace('%episode_number%', (episodeNumber || '').padStart(padding || 0, '0')) .replace('%season_number%', seasonNumber || '') .replace('%episode_title%', title || '') + + if (wordpress.postTitleInput.value != newTitle) { + wordpress.postTitleInput.value = newTitle + + yield postTitleUpdate(newTitle) + } } function* selectMediaFromLibrary(action: { payload: { onSuccess: Action } }) { diff --git a/includes/api/episodes.php b/includes/api/episodes.php index 257b775eb..6a131a9b7 100644 --- a/includes/api/episodes.php +++ b/includes/api/episodes.php @@ -197,6 +197,24 @@ public function register_routes() 'permission_callback' => [$this, 'create_item_permissions_check'], ] ]); + + register_rest_route($this->namespace, '/'.$this->rest_base.'/(?P[\d]+)/build_slug', [ + 'args' => [ + 'id' => [ + 'description' => __('Unique identifier for the episode.', 'podlove-podcasting-plugin-for-wordpress'), + 'type' => 'integer', + ], + 'title' => [ + 'type' => 'string' + ] + ], + [ + 'methods' => \WP_REST_Server::READABLE, + 'callback' => [$this, 'build_slug'], + 'permission_callback' => [$this, 'create_item_permissions_check'], + ] + ]); + register_rest_route($this->namespace, '/'.$this->rest_base.'/(?P[\d]+)', [ 'args' => [ 'id' => [ @@ -665,6 +683,25 @@ public function create_item($request) return new \WP_REST_Response(null, 500); } + public function build_slug($request) + { + $id = $request->get_param('id'); + if (!$id) { + return; + } + + $episode = Episode::find_by_id($id); + if (!$episode) { + return new \Podlove\Api\Error\NotFound(); + } + + $title = $request->get_param('title') ?? get_the_title($episode->post_id); + + $slug = sanitize_title($title); + + return new \Podlove\Api\Response\CreateResponse(['slug' => $slug]); + } + public function update_item_permissions_check($request) { if (!current_user_can('edit_posts')) { From 7e055d4b7a7f70e230b498cfcb12d3ce4b8b5d45 Mon Sep 17 00:00:00 2001 From: Eric Teubert Date: Fri, 2 Feb 2024 19:51:39 +0100 Subject: [PATCH 3/4] simplify --- client/src/sagas/mediafiles.sagas.ts | 35 ++++++++++------------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/client/src/sagas/mediafiles.sagas.ts b/client/src/sagas/mediafiles.sagas.ts index 9ae74ad1d..ddcc78ff0 100644 --- a/client/src/sagas/mediafiles.sagas.ts +++ b/client/src/sagas/mediafiles.sagas.ts @@ -1,6 +1,6 @@ import { PodloveApiClient } from '@lib/api' import { selectors } from '@store' -import { all, call, delay, fork, put, select, takeEvery, throttle } from 'redux-saga/effects' +import { all, call, debounce, fork, put, select, takeEvery, throttle } from 'redux-saga/effects' import * as mediafiles from '@store/mediafiles.store' import * as episode from '@store/episode.store' import * as wordpress from '@store/wordpress.store' @@ -29,8 +29,7 @@ function* initialize(api: PodloveApiClient) { yield takeEvery(mediafiles.DISABLE, handleDisable, api) yield takeEvery(mediafiles.VERIFY, handleVerify, api) yield takeEvery(episode.SAVED, maybeReverify, api) - yield takeEvery(episode.SAVED, maybeUpdateSlug, api) - yield takeEvery(wordpress.UPDATE, maybeUpdateSlugFromWP, api) + yield debounce(2000, wordpress.UPDATE, maybeUpdateSlug, api) yield throttle( 2000, @@ -48,6 +47,10 @@ function* selectMediaFromLibrary() { yield put(wordpress.selectMediaFromLibrary({ onSuccess: { type: mediafiles.SET_UPLOAD_URL } })) } +// NEXT UP: +// - stop autoupdate on manual entry +// - not working: Gutenberg + Blogtitle Autogen (probably better treated as separate issue) + function* setUploadMedia(action: Action) { const url = get(action, ['payload']) const slug = url.split('\\').pop().split('/').pop().split('.').shift() @@ -104,36 +107,22 @@ function* maybeReverify(api: PodloveApiClient, action: { type: string; payload: yield all(mediaFiles.map((file) => call(verifyEpisodeAsset, api, episodeId, file.asset_id))) } -function* maybeUpdateSlug(api: PodloveApiClient, action: { type: string; payload: object }) { - const changedKeys = Object.keys(action.payload) - - const interestingKeys = ['title', 'number'] - const slugNeedsUpdating = changedKeys.some((key) => interestingKeys.includes(key)) - - const episodeId: boolean = yield select(selectors.episode.id) - const postTitle: string = yield select(selectors.post.title) - - if (slugNeedsUpdating) { - const { result } = yield api.get(`episodes/${episodeId}/build_slug`, { - query: { title: postTitle }, - }) - yield put(episode.update({ prop: 'slug', value: result.slug })) - } -} - -function* maybeUpdateSlugFromWP( +function* maybeUpdateSlug( api: PodloveApiClient, action: { type: string; payload: { prop: string; value: any } } ) { const episodeId: boolean = yield select(selectors.episode.id) + const oldSlug: boolean = yield select(selectors.episode.slug) - if (action.payload.prop == 'title') { + if (action.payload.prop == 'title' && action.payload.value) { const newTitle = action.payload.value const { result } = yield api.get(`episodes/${episodeId}/build_slug`, { query: { title: newTitle }, }) - yield put(episode.update({ prop: 'slug', value: result.slug })) + if (oldSlug != result.slug) { + yield put(episode.update({ prop: 'slug', value: result.slug })) + } } } From b273989f6099c61cda2b68fd1040df991ef42c04 Mon Sep 17 00:00:00 2001 From: Eric Teubert Date: Fri, 2 Feb 2024 21:05:29 +0100 Subject: [PATCH 4/4] finish up --- .../modules/mediafiles/components/MediaSlug.vue | 3 +++ client/src/sagas/episode.sagas.ts | 9 ++++++++- client/src/sagas/mediafiles.sagas.ts | 8 +++----- client/src/store/mediafiles.store.ts | 14 +++++++++++++- client/src/store/selectors.ts | 4 ++++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/client/src/modules/mediafiles/components/MediaSlug.vue b/client/src/modules/mediafiles/components/MediaSlug.vue index e4503d0c9..d41822dab 100644 --- a/client/src/modules/mediafiles/components/MediaSlug.vue +++ b/client/src/modules/mediafiles/components/MediaSlug.vue @@ -30,6 +30,7 @@ import { mapState, injectStore } from 'redux-vuex' import { selectors } from '@store' import { update as updateEpisode } from '@store/episode.store' +import { disableSlugAutogen } from '@store/mediafiles.store' export default defineComponent({ setup() { @@ -47,6 +48,8 @@ export default defineComponent({ this.dispatch( updateEpisode({ prop: 'slug', value: (event.target as HTMLInputElement).value }) ) + // disable slug generation on any manual input + this.dispatch(disableSlugAutogen()) }, }, diff --git a/client/src/sagas/episode.sagas.ts b/client/src/sagas/episode.sagas.ts index ee1643357..0fa09ac7a 100644 --- a/client/src/sagas/episode.sagas.ts +++ b/client/src/sagas/episode.sagas.ts @@ -6,6 +6,7 @@ import { debounce, fork, put, select, takeEvery } from 'redux-saga/effects' import { PodloveEpisode } from '../types/episode.types' import * as auphonic from '../store/auphonic.store' import * as episode from '../store/episode.store' +import * as mediafiles from '../store/mediafiles.store' import * as wordpress from '../store/wordpress.store' import { createApi } from './api' import { WebhookConfig } from './auphonic.sagas' @@ -34,9 +35,15 @@ function* updateAuphonicWebhookConfig() { function* initialize(api: PodloveApiClient) { const episodeId: string = yield select(selectors.episode.id) - const { result: episodesResult }: { result: PodloveEpisode } = yield api.get(`episodes/${episodeId}`) + const { result: episodesResult }: { result: PodloveEpisode } = yield api.get( + `episodes/${episodeId}` + ) if (episodesResult) { + if (episodesResult.slug === null) { + yield put(mediafiles.enableSlugAutogen()) + } + yield put(episode.set(episodesResult)) } } diff --git a/client/src/sagas/mediafiles.sagas.ts b/client/src/sagas/mediafiles.sagas.ts index ddcc78ff0..f864a0e30 100644 --- a/client/src/sagas/mediafiles.sagas.ts +++ b/client/src/sagas/mediafiles.sagas.ts @@ -17,6 +17,7 @@ function* mediafilesSaga(): any { function* initialize(api: PodloveApiClient) { const episodeId: string = yield select(selectors.episode.id) + const episodeSlug: string = yield select(selectors.episode.slug) const { result: { results: files }, }: { result: { results: MediaFile[] } } = yield api.get(`episodes/${episodeId}/media`) @@ -47,10 +48,6 @@ function* selectMediaFromLibrary() { yield put(wordpress.selectMediaFromLibrary({ onSuccess: { type: mediafiles.SET_UPLOAD_URL } })) } -// NEXT UP: -// - stop autoupdate on manual entry -// - not working: Gutenberg + Blogtitle Autogen (probably better treated as separate issue) - function* setUploadMedia(action: Action) { const url = get(action, ['payload']) const slug = url.split('\\').pop().split('/').pop().split('.').shift() @@ -113,8 +110,9 @@ function* maybeUpdateSlug( ) { const episodeId: boolean = yield select(selectors.episode.id) const oldSlug: boolean = yield select(selectors.episode.slug) + const enabled: boolean = yield select(selectors.mediafiles.slugAutogenerationEnabled) - if (action.payload.prop == 'title' && action.payload.value) { + if (enabled && action.payload.prop == 'title' && action.payload.value) { const newTitle = action.payload.value const { result } = yield api.get(`episodes/${episodeId}/build_slug`, { diff --git a/client/src/store/mediafiles.store.ts b/client/src/store/mediafiles.store.ts index 77abd5cbf..5d7388167 100644 --- a/client/src/store/mediafiles.store.ts +++ b/client/src/store/mediafiles.store.ts @@ -17,7 +17,7 @@ export type State = { export const initialState: State = { is_initializing: true, - slug_autogeneration_enabled: true, + slug_autogeneration_enabled: false, files: [], } @@ -30,6 +30,8 @@ export const DISABLE = 'podlove/publisher/mediafiles/DISABLE' export const VERIFY = 'podlove/publisher/mediafiles/VERIFY' export const UPLOAD_INTENT = 'podlove/publisher/mediafiles/UPLOAD_INTENT' export const SET_UPLOAD_URL = 'podlove/publisher/mediafiles/SET_UPLOAD_URL' +export const ENABLE_SLUG_AUTOGEN = 'podlove/publisher/mediafiles/ENABLE_SLUG_AUTOGEN' +export const DISABLE_SLUG_AUTOGEN = 'podlove/publisher/mediafiles/DISABLE_SLUG_AUTOGEN' export const init = createAction(INIT) export const initDone = createAction(INIT_DONE) @@ -40,6 +42,8 @@ export const disable = createAction(DISABLE) export const verify = createAction(VERIFY) export const uploadIntent = createAction(UPLOAD_INTENT) export const setUploadUrl = createAction(SET_UPLOAD_URL) +export const enableSlugAutogen = createAction(ENABLE_SLUG_AUTOGEN) +export const disableSlugAutogen = createAction(DISABLE_SLUG_AUTOGEN) // TODO: enable revalidates I think? export const reducer = handleActions( @@ -82,6 +86,14 @@ export const reducer = handleActions( [] ), }), + [ENABLE_SLUG_AUTOGEN]: (state: State): State => ({ + ...state, + slug_autogeneration_enabled: true, + }), + [DISABLE_SLUG_AUTOGEN]: (state: State): State => ({ + ...state, + slug_autogeneration_enabled: false, + }), }, initialState ) diff --git a/client/src/store/selectors.ts b/client/src/store/selectors.ts index 98b89996c..ee355beb2 100644 --- a/client/src/store/selectors.ts +++ b/client/src/store/selectors.ts @@ -113,6 +113,10 @@ const episode = { const mediafiles = { isInitializing: createSelector(root.mediafiles, mediafilesStore.selectors.isInitializing), files: createSelector(root.mediafiles, mediafilesStore.selectors.files), + slugAutogenerationEnabled: createSelector( + root.mediafiles, + mediafilesStore.selectors.slugAutogenerationEnabled + ), } const runtime = {