Disable Lift export when Frontier is empty#3440
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3440 +/- ##
==========================================
+ Coverage 74.64% 74.69% +0.05%
==========================================
Files 285 285
Lines 10999 10999
Branches 1339 1338 -1
==========================================
+ Hits 8210 8216 +6
+ Misses 2406 2400 -6
Partials 383 383
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
4b593ca to
998c218
Compare
imnasnainaec
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @andracc)
src/components/ProjectExport/ExportButton.tsx line 16 at r2 (raw file):
projectId: string; buttonProps?: ButtonProps & { "data-testid"?: string }; disabled?: boolean;
It looks like this new optional prop isn't been used.
src/components/ProjectExport/ExportButton.tsx line 22 at r2 (raw file):
export default function ExportButton(props: ExportButtonProps): ReactElement { const dispatch = useAppDispatch(); const [exports, setExports] = useState<boolean>(false);
The <boolean> can be dropped, since the type is implied from the default value.
src/components/ProjectExport/ExportButton.tsx line 43 at r2 (raw file):
setExports(true); } });
Because setExports is a function that takes a boolean, you should be able to condense this to
await isFrontierNonempty(props.propjectId).then(setExports);
(There's no performance concern for updating a state to what it already is, because rerenders are only triggered when states and props change.)
src/components/ProjectExport/ExportButton.tsx line 53 at r2 (raw file):
<LoadingButton loading={loading} disabled={loading}
Since the LoadingButton has a disabled prop, probably better to update that with loading || !exports than to conflict with it via the buttonProps.
imnasnainaec
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @andracc)
src/components/ProjectExport/ExportButton.tsx line 43 at r2 (raw file):
Previously, imnasnainaec (D. Ror.) wrote…
Because
setExportsis a function that takes a boolean, you should be able to condense this toawait isFrontierNonempty(props.propjectId).then(setExports);(There's no performance concern for updating a state to what it already is, because rerenders are only triggered when states and props change.)
One more possible simplification---since we're not doing any special error handling in this situation, the whole useEffect can be:
useEffect(() => {
isFrontierNonempty(props.projectId).then(setExports);
}, [props.projectId]);
Also, we want the second argument: [props.projectId]. That dependency array makes the useEffect only re-run when the projectId changes (e.g., when the user switches projects from within the project settings).
imnasnainaec
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @andracc)
Resolves #3441
This change is