Skip to content

Commit

Permalink
VDI: fix guest errors with community datasets (#1204)
Browse files Browse the repository at this point in the history
* Handle cases w/ guest user

* Allow guests to view public user datasets

* Show variable tree even if variable id in url does not exist for current study

* Add useValueBasedUpdater hook

* Set pending state synchronously

(cherry picked from commit b14db0c)
  • Loading branch information
dmfalke committed Sep 17, 2024
1 parent 0182918 commit f4571f7
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 26 deletions.
12 changes: 9 additions & 3 deletions packages/libs/eda/src/lib/core/hooks/promise.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useEffect, useState } from 'react';
import { useValueBasedUpdater } from './state';

export type PromiseHookState<T> = {
value?: T;
Expand Down Expand Up @@ -28,13 +29,18 @@ export function usePromise<T>(
const [state, setState] = useState<PromiseHookState<T>>({
pending: true,
});
useEffect(() => {
let ignoreResolve = false;

// Set pending state synchronously
useValueBasedUpdater(task, () => {
setState((prev) => ({
pending: true,
value: keepPreviousValue ? prev.value : undefined,
error: undefined,
}));
});

useEffect(() => {
let ignoreResolve = false;
task().then(
(value) => {
if (ignoreResolve) return;
Expand All @@ -54,7 +60,7 @@ export function usePromise<T>(
return function cleanup() {
ignoreResolve = true;
};
}, [keepPreviousValue, task]);
}, [task]);
if (state.error && throwError) throw state.error;
return state;
}
12 changes: 12 additions & 0 deletions packages/libs/eda/src/lib/core/hooks/state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useRef } from 'react';

/**
* Calls the `updater` function when `value` changes.
*/
export function useValueBasedUpdater<T>(value: T, updater: () => void) {
const ref = useRef<T>();
if (ref.current !== value) {
updater();
}
ref.current = value;
}
3 changes: 2 additions & 1 deletion packages/libs/eda/src/lib/workspace/AllAnalyses.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,9 @@ export function AllAnalyses(props: Props) {
const [ownUserDatasets, communityDatasets] =
useWdkService(async (wdkService) => {
if (isVdiCompatibleWdkService(wdkService)) {
const user = await wdkService.getCurrentUser();
return Promise.all([
wdkService.getCurrentUserDatasets(),
user.isGuest ? [] : wdkService.getCurrentUserDatasets(),
wdkService.getCommunityDatasets(),
]);
}
Expand Down
35 changes: 17 additions & 18 deletions packages/libs/eda/src/lib/workspace/Subsetting/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,15 @@ export default function Subsetting({
[entity, variable]
);

if (
entity == null ||
(!Variable.is(variable) && !MultiFilterVariable.is(variable))
) {
return <div>Could not find specified variable.</div>;
}

if (multiFilterParent) {
return <Redirect to={multiFilterParent.id} />;
}

const totalEntityCount = totalCounts && totalCounts[entity.id];
const totalEntityCount = totalCounts && entity && totalCounts[entity.id];

// This will give you the count of rows for the current entity.
const filteredEntityCount = filteredCounts && filteredCounts[entity.id];
const filteredEntityCount =
filteredCounts && entity && filteredCounts[entity.id];

const starredVariables = analysisState.analysis?.descriptor.starredVariables;

Expand All @@ -88,22 +82,27 @@ export default function Subsetting({
<div className="Variables">
<VariableTree
scope={scope}
entityId={entity.id}
entityId={entity?.id}
starredVariables={starredVariables}
toggleStarredVariable={toggleStarredVariable}
variableId={variable.id}
variableId={variable?.id}
variableLinkConfig={variableLinkConfig}
/>
</div>

<div className="Filter">
<VariableDetails
entity={entity}
variable={variable}
analysisState={analysisState}
totalEntityCount={totalEntityCount}
filteredEntityCount={filteredEntityCount}
/>
{entity == null ||
(!Variable.is(variable) && !MultiFilterVariable.is(variable)) ? (
<div>Could not find specified variable in this study.</div>
) : (
<VariableDetails
entity={entity}
variable={variable}
analysisState={analysisState}
totalEntityCount={totalEntityCount}
filteredEntityCount={filteredEntityCount}
/>
)}
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ class UserDatasetDetailController extends PageController<MergedProps> {
const entry = userDatasetsById[id];
const isOwner = !!(
user &&
entry &&
entry.resource &&
entry.resource.ownerUserId === user.id
);
Expand All @@ -237,8 +238,8 @@ class UserDatasetDetailController extends PageController<MergedProps> {
sharingSuccess,
shareSuccessful,
updateSharingModalState,
userDataset: entry.resource,
fileListing: entry.fileListing,
userDataset: entry?.resource,
fileListing: entry?.fileListing,
getQuestionUrl: this.getQuestionUrl,
questionMap: keyBy(questions, 'fullName'),
workspaceTitle,
Expand All @@ -254,9 +255,11 @@ class UserDatasetDetailController extends PageController<MergedProps> {
};

const DetailView = this.getDetailView(
typeof entry.resource === 'object' ? entry.resource.type : null
typeof entry?.resource === 'object' ? entry.resource.type : null
);
return user && user.isGuest ? (
return entry?.resource?.meta.visibility !== 'public' &&
user &&
user.isGuest ? (
this.renderGuestView()
) : (
<DetailView {...props} />
Expand Down

0 comments on commit f4571f7

Please sign in to comment.