the list refreshes before the message, and the tab follows the result
reported as "the list does not update without a refresh". the counts were in fact correct, which is the more interesting half: fofos-admin was ALREADY INSTALLED on that host, so the fetch put it in Installed while the confirmation said "click it to install" and pointed at an empty Available tab. right numbers, wrong story. three fixes, and only one of them is the refresh. AWAIT THE REFETCH. `invalidateQueries` is fire-and-forget, so the confirmation appeared while the grid was still rendering the previous answer and the plugin turned up a moment later. that reads as "nothing happened" and sends somebody looking for a refresh button — which is exactly what happened. SAY WHICH THING HAPPENED. a directory and an install row are separate facts: deleting a plugin's source leaves the row, so a re-fetch can land on something the platform still considers installed. the route now reports it, and the message is either "added — click it to install" or "added — already installed". FOLLOW IT TO ITS TAB. if it came back installed, the overview switches to Installed rather than leaving somebody staring at the tab it is not in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -8,6 +8,7 @@ import { fetchCatalogue, fetchReadme } from '../../plugins/marketplace';
|
|||||||
import { findOrphanTables } from '../../plugins/orphans';
|
import { findOrphanTables } from '../../plugins/orphans';
|
||||||
import { fetchPlugin } from '../../plugins/fetch';
|
import { fetchPlugin } from '../../plugins/fetch';
|
||||||
import { discoverPlugins } from '../../plugins/discover';
|
import { discoverPlugins } from '../../plugins/discover';
|
||||||
|
import { listPluginInstalls } from 'officerdb';
|
||||||
import { iconUrl } from '../../app-store/assets';
|
import { iconUrl } from '../../app-store/assets';
|
||||||
import {
|
import {
|
||||||
installPlugin,
|
installPlugin,
|
||||||
@@ -309,7 +310,18 @@ pluginsRouter.post('/fetch-from-url', async (ctx) => {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return ctx.json({ ok: true, appName, label: found.manifest?.label ?? appName });
|
/*
|
||||||
|
Whether it is ALREADY installed, so the UI can say the true thing and show the right tab.
|
||||||
|
|
||||||
|
A directory and an install row are separate facts — deleting a plugin's source leaves the row, so a
|
||||||
|
re-fetch can land on something the platform still considers installed. Reported here because the
|
||||||
|
caller has just been told "added" and needs to know where to look; "click it to install" pointing at
|
||||||
|
an empty tab is worse than no message.
|
||||||
|
*/
|
||||||
|
const installs = await listPluginInstalls();
|
||||||
|
const installed = installs.some((entry) => entry.appName === appName && entry.enabled);
|
||||||
|
|
||||||
|
return ctx.json({ ok: true, appName, label: found.manifest?.label ?? appName, installed });
|
||||||
});
|
});
|
||||||
|
|
||||||
pluginsRouter.post('/:appName/install', async (ctx) => {
|
pluginsRouter.post('/:appName/install', async (ctx) => {
|
||||||
|
|||||||
@@ -27,7 +27,7 @@ import { GitBranch, X, Loader2 } from 'lucide-react';
|
|||||||
export const InstallFromUrl = ({
|
export const InstallFromUrl = ({
|
||||||
onFetch,
|
onFetch,
|
||||||
}: {
|
}: {
|
||||||
onFetch: (url: string) => Promise<{ ok: boolean; appName?: string; error?: string }>;
|
onFetch: (url: string) => Promise<{ ok: boolean; appName?: string; installed?: boolean; error?: string }>;
|
||||||
}) => {
|
}) => {
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
const [url, setUrl] = useState('');
|
const [url, setUrl] = useState('');
|
||||||
@@ -62,9 +62,18 @@ export const InstallFromUrl = ({
|
|||||||
const result = await onFetch(trimmed);
|
const result = await onFetch(trimmed);
|
||||||
setBusy(false);
|
setBusy(false);
|
||||||
if (result.ok) {
|
if (result.ok) {
|
||||||
// Named, because the name was derived rather than typed — this is where you find out what to look
|
/*
|
||||||
// for in the grid.
|
Named, because the name was derived rather than typed — this is where you find out what to look
|
||||||
setNote({ ok: true, text: `${result.appName} added — click it to install` });
|
for. And it says which of the two things happened: a plugin whose source was deleted but whose
|
||||||
|
install row survived comes back ALREADY installed, and telling somebody to "click it to install"
|
||||||
|
while it sits in a different tab is worse than saying nothing.
|
||||||
|
*/
|
||||||
|
setNote({
|
||||||
|
ok: true,
|
||||||
|
text: result.installed
|
||||||
|
? `${result.appName} added — already installed`
|
||||||
|
: `${result.appName} added — click it to install`,
|
||||||
|
});
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
setUrl('');
|
setUrl('');
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ export const MarketplaceOverview = ({
|
|||||||
official: Entry[];
|
official: Entry[];
|
||||||
available: Entry[];
|
available: Entry[];
|
||||||
renderCard: (entry: Entry) => ReactNode;
|
renderCard: (entry: Entry) => ReactNode;
|
||||||
onFetch: (url: string) => Promise<{ ok: boolean; appName?: string; error?: string }>;
|
onFetch: (url: string) => Promise<{ ok: boolean; appName?: string; installed?: boolean; error?: string }>;
|
||||||
}) => {
|
}) => {
|
||||||
// Opens on what you HAVE. A store that opens on the catalogue is selling; this is mostly a place
|
// Opens on what you HAVE. A store that opens on the catalogue is selling; this is mostly a place
|
||||||
// people come to check on something they already installed.
|
// people come to check on something they already installed.
|
||||||
@@ -74,7 +74,16 @@ export const MarketplaceOverview = ({
|
|||||||
you cannot see, which is the mistake this whole section is correcting. */}
|
you cannot see, which is the mistake this whole section is correcting. */}
|
||||||
{tab === 'available' && (
|
{tab === 'available' && (
|
||||||
<div className="ml-auto flex min-w-0 flex-1 justify-end py-1.5 pl-3">
|
<div className="ml-auto flex min-w-0 flex-1 justify-end py-1.5 pl-3">
|
||||||
<InstallFromUrl onFetch={onFetch} />
|
{/* The tab FOLLOWS the result. A plugin whose directory was removed but whose install row
|
||||||
|
survived comes back already installed, so "click it to install" would be pointing at a
|
||||||
|
tab it is not in — which is exactly what happened the first time this shipped. */}
|
||||||
|
<InstallFromUrl
|
||||||
|
onFetch={async (url) => {
|
||||||
|
const result = await onFetch(url);
|
||||||
|
if (result.ok && result.installed) setTab('installed');
|
||||||
|
return result;
|
||||||
|
}}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -288,18 +288,30 @@ export function usePlugins() {
|
|||||||
* have already used.
|
* have already used.
|
||||||
*/
|
*/
|
||||||
const fetchFromUrl = useCallback(
|
const fetchFromUrl = useCallback(
|
||||||
async (url: string): Promise<{ ok: boolean; appName?: string; error?: string }> => {
|
async (url: string): Promise<{ ok: boolean; appName?: string; installed?: boolean; error?: string }> => {
|
||||||
const res = await fetch('/api/plugins/fetch-from-url', {
|
const res = await fetch('/api/plugins/fetch-from-url', {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: { ...getHeaders(), 'content-type': 'application/json' },
|
headers: { ...getHeaders(), 'content-type': 'application/json' },
|
||||||
body: JSON.stringify({ url }),
|
body: JSON.stringify({ url }),
|
||||||
});
|
});
|
||||||
const body = (await res.json().catch(() => ({}))) as { appName?: string; error?: string };
|
const body = (await res.json().catch(() => ({}))) as {
|
||||||
// Refresh either way: a clone that failed discovery still left something on disk worth showing.
|
appName?: string;
|
||||||
|
installed?: boolean;
|
||||||
|
error?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
/*
|
||||||
|
AWAITED, so the caller resolves after the list has actually been refetched.
|
||||||
|
`invalidateQueries` is fire-and-forget: without awaiting it, the confirmation appeared while the
|
||||||
|
grid was still showing the previous answer, and the new plugin turned up a moment later — which
|
||||||
|
reads as "nothing happened" and sends somebody looking for a refresh button.
|
||||||
|
*/
|
||||||
|
await queryClient.invalidateQueries({ queryKey: PLUGINS_KEY });
|
||||||
|
// A clone that failed discovery still left something on disk worth showing, so this runs either way.
|
||||||
invalidate();
|
invalidate();
|
||||||
return { ok: res.ok, appName: body.appName, error: body.error };
|
return { ok: res.ok, appName: body.appName, installed: body.installed, error: body.error };
|
||||||
},
|
},
|
||||||
[invalidate],
|
[invalidate, queryClient],
|
||||||
);
|
);
|
||||||
|
|
||||||
// Update-all is a PLAIN POST, not the streaming door the single verbs use. Twelve interleaved step
|
// Update-all is a PLAIN POST, not the streaming door the single verbs use. Twelve interleaved step
|
||||||
|
|||||||
Reference in New Issue
Block a user