Skip to content

Commit

Permalink
Fix for broken model load on private LFS from break in OPFS, also Ver…
Browse files Browse the repository at this point in the history
…sions would have been broken for using finalUrl (LFS URL) for branch path composition. Fix Notes/Octokit race on non/authed init.
  • Loading branch information
pablo-mayrgundter committed Feb 22, 2024
1 parent 246f33d commit e494c1a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 14 deletions.
11 changes: 11 additions & 0 deletions src/Components/Notes/NotesContol.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ export default function NotesControl() {

// Fetch issues/notes
useEffect(() => {
// TODO(pablo): NotesControl loads onViewer, bc viewer for non-logged in
// session is valid. But! When the model is private, there's a delayed
// load until after auth succeeds. If we don't check model here, then Notes
// initially fails during an unauthenticated load via oauthproxy, which gets
// a 302 DIY, and somehow seems to keep that state in Octokit.
//
// We detect we're in a delayed load state here by checking model first,
// which then doesn't touch octokit until later when auth is available.
if (!model) {
return
}
(async () => {
toggleIsLoadingNotes()
try {
Expand Down
21 changes: 7 additions & 14 deletions src/Containers/CadView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ export default function CadView({
}

const pathToLoad = modelPath.gitpath || (installPrefix + modelPath.filepath)
const tmpModelRef = await loadIfc(pathToLoad)
const tmpModelRef = await loadIfc(pathToLoad, modelPath.gitpath)

if (tmpModelRef === 'redirect') {
return
Expand Down Expand Up @@ -383,8 +383,9 @@ export default function CadView({
* Load IFC helper used by 1) useEffect on path change and 2) upload button.
*
* @param {string} filepath
* @param {string} gitpath to use for constructing API endpoints
*/
async function loadIfc(filepath) {
async function loadIfc(filepath, gitpath) {
debug().log(`CadView#loadIfc: `, filepath)
const uploadedFile = pathPrefix.endsWith('new')

Expand All @@ -398,6 +399,8 @@ export default function CadView({
setIsModelLoading(true)
setSnackMessage(`${loadingMessageBase}`)

// NB: for LFS targets, this will now be media.githubusercontent.com, so
// don't use for further API endpoint construction.
const ifcURL = (uploadedFile || filepath.indexOf('/') === 0) ? filepath : await getFinalURL(filepath, accessToken)

let loadedModel
Expand Down Expand Up @@ -502,8 +505,7 @@ export default function CadView({
} else {
// TODO(pablo): probably already available in this scope, or use
// parseGitHubRepositoryURL instead.
const url = new URL(ifcURL)
const {isPublic, owner, repo, branch, filePath} = parseGitHubPath(url.pathname)
const {isPublic, owner, repo, branch, filePath} = parseGitHubPath(new URL(gitpath).pathname)
const commitHash = isPublic ?
await getLatestCommitHash(owner, repo, filePath, '', branch) :
await getLatestCommitHash(owner, repo, filePath, accessToken, branch)
Expand Down Expand Up @@ -806,15 +808,6 @@ export default function CadView({
}
}

// TODO(pablo): again, just need branch here for VersionsContainer
// below. It's probably already available in this scope.
let ghPath = location.pathname
if (ghPath.startsWith(`${appPrefix}/v/gh`)) {
ghPath = ghPath.substring(`${appPrefix}/v/gh`.length)
}
const {branch} = parseGitHubPath(ghPath)


const windowDimensions = useWindowDimensions()
const spacingBetweenSearchAndOpsGroupPx = 20
const operationsGroupWidthPx = 100
Expand Down Expand Up @@ -931,7 +924,7 @@ export default function CadView({
modelPath.repo !== undefined && isVersionHistoryVisible &&
<VersionsContainer
filePath={modelPath.filepath}
currentRef={branch}
currentRef={modelPath.branch}
/>
}
</Box>
Expand Down

0 comments on commit e494c1a

Please sign in to comment.