Skip to content

Commit

Permalink
Solve race condition on update branch (#1571)
Browse files Browse the repository at this point in the history
* solving race condition

* fixed effect dep

* ensure branch check after save
  • Loading branch information
abelpz authored Apr 26, 2023
1 parent 21bd958 commit e6e5da8
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 14 deletions.
2 changes: 1 addition & 1 deletion public/build_number
Original file line number Diff line number Diff line change
@@ -1 +1 @@
251-fcff0e4
254-bb7a029
9 changes: 7 additions & 2 deletions src/components/branch-merger/hooks/useBranchMerger.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,13 @@ export default function useBranchMerger({ server, owner, repo, userBranch, token
const checkUpdateStatus = useCallback(() => {
setLoadingUpdate(true);
const setter = setUpdateStatus;
const handler = checkUpdate
return runMergeHandler({setter,handler,params}).then(r => { setLoadingUpdate(false); return r})
const handler = checkUpdate;
console.log("branch-merger: started checking update");
return runMergeHandler({setter,handler,params}).then(r => {
setLoadingUpdate(false);
console.log("branch-merger: finished checking update");
return r
})
},[params,runMergeHandler])

/**
Expand Down
5 changes: 4 additions & 1 deletion src/components/translatable/Translatable.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ function Translatable() {
} else if (filepath.match(/\.tsv$/)) {
console.log('TSV file selected');
const onSave = async function (...args) {
return await saveTranslation(...args);
console.log("branch-merger: started saving");
const saved = await saveTranslation(...args);
console.log("branch-merger: finished saving");
return saved;
}
const onEdit = function (...args) {
autoSaveOnEdit(...args);
Expand Down
2 changes: 1 addition & 1 deletion src/components/translatable/TranslatableTSV.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export default function TranslatableTSV({
return _config;
}, [columnNames, rowHeader]);

const updateButtonProps = useContentUpdateProps();
const updateButtonProps = useContentUpdateProps({isSaving});
const {
isErrorDialogOpen,
onCloseErrorDialog,
Expand Down
16 changes: 7 additions & 9 deletions src/hooks/useContentUpdateProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useContext, useEffect, useMemo, useState } from 'react'
import { AppContext } from '../App.context';
import { BranchMergerContext } from '../components/branch-merger/context/BranchMergerProvider';

export function useContentUpdateProps({isLoading: _isLoading = false} = {}) {
export function useContentUpdateProps({isLoading: _isLoading = false, isSaving = false} = {}) {
const [isLoading, setIsLoading] = useState(_isLoading);
const [isErrorDialogOpen,setIsErrorDialogOpen] = useState(false);

Expand All @@ -20,18 +20,16 @@ export function useContentUpdateProps({isLoading: _isLoading = false} = {}) {
const loadingProps = { color: loadingUpdate ? "primary" : "secondary" };

const { load: loadTargetFile } = targetFileHook.actions || {};
const { publishedContent } = targetFileHook.state || {};

useEffect(() => {
/* publishedContent is undefined when the content is being saved
and an empty string when the content has finished saving. */
if(publishedContent !== undefined) {
setIsLoading(false);
} else {
if(isSaving & !isLoading) {
setIsLoading(true);
checkUpdateStatus(); //check after content is being saved and reloaded
}
},[publishedContent, checkUpdateStatus])
if(!isSaving & isLoading) {
checkUpdateStatus();
setIsLoading(false);
}
},[isSaving,isLoading,checkUpdateStatus])

const {conflict,mergeNeeded,error,message,pullRequest} = updateStatus
const pending = mergeNeeded || conflict
Expand Down

0 comments on commit e6e5da8

Please sign in to comment.