Add related pub button

[allisonking]

Issue(s) Resolved

Closes #953

High-level Explanation of PR

TODO

• TS error on slug for ConfigureRelatedValue
• Adding a related pub currently replaces any current related value instead of appending—check if new update/upsert/create logic might address this?
• Check file upload, member select, context editor and other tricky form values work ok
• Refactor editorSpecifiers
• Tests

Test Plan

1. Add any related field (null should also work)
2. Add the field to a pub type
3. Create a pub with that pub type
4. On the created pub page, click the "Add related pub" button
5. You should get a modal that lets you choose a pubtype, relationship field, and relationship value
6. Click "Create pub"
7. This should bring you to the create pub page. Create a pub here
8. Go back to the pub created in step 3 and verify that the pub created in step 7 is now in the "Related pubs" table. You should also see the field it was related on and the value.

Screenshots (if applicable)

Notes

[allisonking]

I'm not totally sure that this is safe—basically the problem is that our "create pub" flow is split across two views. The first view selects the pub type, and now also the relation definition. Then it redirects to the full create pub page, passing along its info via query parameters.

Is there any reason we wouldn't want the value field of the relation to be passed via query param to the other page? I think perhaps it could get long? or in the case of file upload, the path to the s3 file would end up in the query param i.e. http://localhost:3000/c/croccroc/pubs/create?pubTypeId=c8be5cf7-3fd0-47f1-8ddc-eb954526efa3&relatedPubId=1a5a3582-01b6-45b1-8ad5-1bd6019289ab&slug=croccroc%3Arelated-file&value=%5B%7B%22id%22%3A%22relatedPub.value-test%2Ffile%2Ftxt-1d-1e-text%2Fplain-11-1722373001847%22%2C%22fileName%22%3A%22test-file.txt%22%2C%22fileSource%22%3A%22dashboard-relatedPub.value%22%2C%22fileType%22%3A%22text%2Fplain%22%2C%22fileSize%22%3A11%2C%22fileMeta%22%3A%7B%22relativePath%22%3Anull%2C%22name%22%3A%22test-file.txt%22%2C%22type%22%3A%22text%2Fplain%22%7D%2C%22fileUploadUrl%22%3A%22https%3A%2F%2Fs3.us-east-1.amazonaws.com%2Fassets.v7.pubpub.org%2F1a5a3582-01b6-45b1-8ad5-1bd6019289ab%2Ftest-file.txt%22%7D%5D

[allisonking]

the alternative would be to store the value in something like local storage, or maybe a context?

[tefkah]

hmmm yeah i think this does show the limits of this approach. i think we should probably not send the value over the url. if you for instance send a medium-sized RichText value there it's very likely you go over the url length of 2000ish characters.

hmm, mayyybe a context could work, but that would be a bit messy i think. i think local-storage would probably be a better idea, using this hook

platform/packages/ui/src/hooks/useLocalStorage.tsx

Lines 1 to 57 in e6ea040

	"use client";
	import * as React from "react";
	export type LocalStorageContext = {
	prefix?: string;
	timeout?: number;
	};
	export const LocalStorageContext = React.createContext<Record<string, any>>({
	prefix: "",
	timeout: 0,
	});
	export const LocalStorageProvider = (props: React.PropsWithChildren<LocalStorageContext>) => {
	return (
	<LocalStorageContext.Provider value={props}>{props.children}</LocalStorageContext.Provider>
	);
	};
	export const useLocalStorage = <T,>(key: string): [T | undefined, (value: T) => void] => {
	const { prefix = "", timeout } = React.useContext(LocalStorageContext);
	key = React.useMemo(() => prefix + key, []);
	const timestamp = React.useRef(performance.now());
	const value = React.useMemo<T | undefined>(() => {
	if (typeof localStorage === "undefined") {
	return undefined;
	}
	const item = localStorage.getItem(key);
	if (item) {
	return JSON.parse(item);
	}
	return undefined;
	}, []);
	const tail = React.useRef<T | null>(null);
	const tailTimer = React.useRef<ReturnType<typeof setTimeout> | null>(null);
	const setValue = React.useCallback(
	(value: T) => {
	const now = performance.now();
	if (tailTimer.current) {
	clearTimeout(tailTimer.current);
	}
	if (!timeout || now - timestamp.current < timeout) {
	timestamp.current = now;
	localStorage.setItem(key, JSON.stringify(value));
	} else {
	tail.current = value;
	tailTimer.current = setTimeout(() => {
	timestamp.current = now;
	tailTimer.current = null;
	localStorage.setItem(key, JSON.stringify(value));
	}, timeout);
	}
	},
	[key, timeout]
	);
	return [value, setValue];
	};

if you store it in something like ${currentPubId}-temp-related-value-${fieldSlug}-${newPubId} i think it could be fine. Then you can just send that key over in the searchparams. Still, kind of ugly solution.

In general though i feel like this flow is kind of strange. I was thinking that maybe using intercepting routes this could be easier, but no you would still need to send that data through the URL somehow.

Something i've been thinking about is that it would be nice if we could have Pubs in some kind of "draft" or "not really done yet" state. That way we could just do the normal thing and create this pub in this draft stage + create the value, then redirect the user to complete the pub. Only once the user "creates" the pub is the whole thing actually "committed" (not sure what that looks like). This would also make eg the autosave feature for forms behave in a more "expected" way (to me). I guess ofc we can ofc already sort of do the draft state thing manually with Stages as statuses, but it feels a bit manual.

Hmmm, tough one! I think if we for now do it through the localstorage that's good enough, but given how long we stick with "good enough" it would be nice if we could figure out a more sustainable solution!

[tefkah]

@3mcd @kalilsn would love your thoughts here as well!

[tefkah]

another option (which is maybe nicer i think) would be to, instead of configuring the value on the Pubs page, we add that value configuration (somehow, somewhere) to the Pub Create page.

so instead of going to /pubs/create?value={very big thing}, you go to /pubs/create?relatedField=${fieldSlug}. then we figure out what kind of field relatedField is, and render somewhere ??? , maybe in the header, that value config?

[tefkah]

Nevermind! I checked a really old source, apparently modern browsers can easily handle >64k characters in the url (https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers). That should be enough for most cases, we can probably just put a limit on the max length of related value fields to prevent that.
Given how simple it is, i think passing the value through the URL is fine enough. maybe encode it in 64bit just to make it look slightly nicer, but that's not really necessary.

[allisonking]

I could use some TS suggestions here—right now ConfigureRelatedValue is typed such that it expects to be part of an array field, but that's not the case here, where it's actually the only field (slug is expected to be i.e. field.0.value when really this one is just field.value. the strict field.0.value typing in ConfigureRelatedValue is to help render field array error states)

[tefkah]

ah tricky one!

i'd try something like this in RelatedPubsElement.tsx

type FormValue = {
	[slug: string]: FieldValue[];
};
type FormValueSingle = {
	[slug: string]: FieldValue;
};

const parseRelatedPubValueSlugError = (
	slug: RelatedPubValueSlug,
	formStateErrors: FieldErrors<FormValueSingle> | FieldErrors<FormValue>
) => {
	const [baseSlug, index] = slug.split(".");
	const indexNumber = index ? parseInt(index) : undefined;

	if (!indexNumber || isNaN(indexNumber)) {
		const baseError = (formStateErrors as FieldErrors<FormValueSingle>)[baseSlug];
		return baseError?.value;
	}

	const valueError = (formStateErrors as FieldErrors<FormValue>)[baseSlug]?.[indexNumber]?.value;

	return valueError;
};

// this is existing code
export const ConfigureRelatedValue = ({
	slug,
	element,
	onBlur,
	className,
	...props
}: PubFieldFormElementProps & {
	slug: RelatedPubValueSlug;
	onBlur?: () => void;
	className?: string;
}) => {
	const configLabel = "label" in element.config ? element.config.label : undefined;
	const label = configLabel || element.label || slug;

	const { watch, formState } = useFormContext<{
		[slug: string]: FieldValue[] | FieldValue;
	}>();
	const [isPopoverOpen, setPopoverIsOpen] = useState(false);
	const value = watch(slug);
	const showValue = value != null && value !== "";

// new 
	const valueError = parseRelatedPubValueSlugError(slug, formState.errors);

[tefkah]

it works well! other than that minor suggestion about when to show the related pubs button/table, the question about how to pass the value to the create form is very tricky!

i think i'd like to hear @3mcd or @kalilsn 's take on this! i won't press request changes as that'll block the pr! sorry, bit in a hurry as i wanted to get this in before i signed off!

[tefkah]

hmmm yeah i think this does show the limits of this approach. i think we should probably not send the value over the url. if you for instance send a medium-sized RichText value there it's very likely you go over the url length of 2000ish characters.

hmm, mayyybe a context could work, but that would be a bit messy i think. i think local-storage would probably be a better idea, using this hook

platform/packages/ui/src/hooks/useLocalStorage.tsx

Lines 1 to 57 in e6ea040

	"use client";
	import * as React from "react";
	export type LocalStorageContext = {
	prefix?: string;
	timeout?: number;
	};
	export const LocalStorageContext = React.createContext<Record<string, any>>({
	prefix: "",
	timeout: 0,
	});
	export const LocalStorageProvider = (props: React.PropsWithChildren<LocalStorageContext>) => {
	return (
	<LocalStorageContext.Provider value={props}>{props.children}</LocalStorageContext.Provider>
	);
	};
	export const useLocalStorage = <T,>(key: string): [T | undefined, (value: T) => void] => {
	const { prefix = "", timeout } = React.useContext(LocalStorageContext);
	key = React.useMemo(() => prefix + key, []);
	const timestamp = React.useRef(performance.now());
	const value = React.useMemo<T | undefined>(() => {
	if (typeof localStorage === "undefined") {
	return undefined;
	}
	const item = localStorage.getItem(key);
	if (item) {
	return JSON.parse(item);
	}
	return undefined;
	}, []);
	const tail = React.useRef<T | null>(null);
	const tailTimer = React.useRef<ReturnType<typeof setTimeout> | null>(null);
	const setValue = React.useCallback(
	(value: T) => {
	const now = performance.now();
	if (tailTimer.current) {
	clearTimeout(tailTimer.current);
	}
	if (!timeout || now - timestamp.current < timeout) {
	timestamp.current = now;
	localStorage.setItem(key, JSON.stringify(value));
	} else {
	tail.current = value;
	tailTimer.current = setTimeout(() => {
	timestamp.current = now;
	tailTimer.current = null;
	localStorage.setItem(key, JSON.stringify(value));
	}, timeout);
	}
	},
	[key, timeout]
	);
	return [value, setValue];
	};

if you store it in something like ${currentPubId}-temp-related-value-${fieldSlug}-${newPubId} i think it could be fine. Then you can just send that key over in the searchparams. Still, kind of ugly solution.

In general though i feel like this flow is kind of strange. I was thinking that maybe using intercepting routes this could be easier, but no you would still need to send that data through the URL somehow.

Something i've been thinking about is that it would be nice if we could have Pubs in some kind of "draft" or "not really done yet" state. That way we could just do the normal thing and create this pub in this draft stage + create the value, then redirect the user to complete the pub. Only once the user "creates" the pub is the whole thing actually "committed" (not sure what that looks like). This would also make eg the autosave feature for forms behave in a more "expected" way (to me). I guess ofc we can ofc already sort of do the draft state thing manually with Stages as statuses, but it feels a bit manual.

Hmmm, tough one! I think if we for now do it through the localstorage that's good enough, but given how long we stick with "good enough" it would be nice if we could figure out a more sustainable solution!

[tefkah]

@3mcd @kalilsn would love your thoughts here as well!

[tefkah]

another option (which is maybe nicer i think) would be to, instead of configuring the value on the Pubs page, we add that value configuration (somehow, somewhere) to the Pub Create page.

so instead of going to /pubs/create?value={very big thing}, you go to /pubs/create?relatedField=${fieldSlug}. then we figure out what kind of field relatedField is, and render somewhere ??? , maybe in the header, that value config?

[tefkah]

Nevermind! I checked a really old source, apparently modern browsers can easily handle >64k characters in the url (https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers). That should be enough for most cases, we can probably just put a limit on the max length of related value fields to prevent that.
Given how simple it is, i think passing the value through the URL is fine enough. maybe encode it in 64bit just to make it look slightly nicer, but that's not really necessary.

[tefkah]

ah tricky one!

i'd try something like this in RelatedPubsElement.tsx

type FormValue = {
	[slug: string]: FieldValue[];
};
type FormValueSingle = {
	[slug: string]: FieldValue;
};

const parseRelatedPubValueSlugError = (
	slug: RelatedPubValueSlug,
	formStateErrors: FieldErrors<FormValueSingle> | FieldErrors<FormValue>
) => {
	const [baseSlug, index] = slug.split(".");
	const indexNumber = index ? parseInt(index) : undefined;

	if (!indexNumber || isNaN(indexNumber)) {
		const baseError = (formStateErrors as FieldErrors<FormValueSingle>)[baseSlug];
		return baseError?.value;
	}

	const valueError = (formStateErrors as FieldErrors<FormValue>)[baseSlug]?.[indexNumber]?.value;

	return valueError;
};

// this is existing code
export const ConfigureRelatedValue = ({
	slug,
	element,
	onBlur,
	className,
	...props
}: PubFieldFormElementProps & {
	slug: RelatedPubValueSlug;
	onBlur?: () => void;
	className?: string;
}) => {
	const configLabel = "label" in element.config ? element.config.label : undefined;
	const label = configLabel || element.label || slug;

	const { watch, formState } = useFormContext<{
		[slug: string]: FieldValue[] | FieldValue;
	}>();
	const [isPopoverOpen, setPopoverIsOpen] = useState(false);
	const value = watch(slug);
	const showValue = value != null && value !== "";

// new 
	const valueError = parseRelatedPubValueSlugError(slug, formState.errors);

[3mcd]

@tefkah @allisonking I definitely like Thomas's suggestion to move the configuration of the related pub field value to the create form somehow, if it wouldn't be too much work. Maybe spend an hour or two on it, and if it doesn't work out in that time, we can stick with the URL serialization and create a card for it.

[allisonking]

Update—related value is now on the pub create page