Restore restore-file feature
#6171
Conversation
fregante
changed the title
restore-file featurerestore-file feature
|
Did you verify whether this works on PR from forks? This was the issue last time we tried |
|
This doesn't work when the file contains non-ascii characters unfortunately:
Screen.Recording.1.movSee https://stackoverflow.com/a/54302528 I think you should be able to test it on this PR, once fixed, exactly like in my demo. If the file is restored correctly, then we can merge it! |
|
It's unclear whether this would work correctly with GitHub: https://developer.mozilla.org/en-US/docs/Web/API/btoa#unicode_strings GitHub’s docs don't go into detail on how to deal with this either: https://docs.github.com/en/rest/repos/contents#create-or-update-file-contents I wonder if something as simple as this is enough for GitHub: https://stackoverflow.com/a/53221307 |
|
I'm confused as to why we're getting a raw text string and pushing a base64. There must be a way to avoid having to do any conversion. In my first iteration, I was using the API v3 for this, but I had to turn to |
Yes, it works - dabbu-knowledge-platform/test#1 |
|
I don't see the Restore File button for images :/
|
|
Turns out its because the "Edit file" button is disabled (L120), which makes sense to me. |
|
I can't play the video you shared:
But it does seem to work with non-ascii characters: dabbu-knowledge-platform/test@776b841 |
|
The non-ASCII files need to be in the previous version, not the "new" one. We fetch and submit the previous version in order to restore it. Try on this PR, I tried restoring the last file in the PR Files list. As for the video, you can download it and open it with VLC if the browser won't play it |
|
@gamemaker1 can you finalize this PR? I'd love to ship it this week |
gamemaker1
force-pushed
the
fix/restore-file
branch
from
26d0558
to
26b9652
Compare
|
Done :) |
|
Neat! I think it works, tested in Chrome and Firefox: refined-github/sandbox#54 Will merge and release a new version soon |
| const {repository} = await api.v4(` | ||
| repository() { # Cache buster ${Math.random()} | ||
| pullRequest(number: ${getConversationNumber()!}) { | ||
| headRefOid |
There was a problem hiding this comment.
Can you avoid this separate call? This specific line just needs to be moved inside getBaseReference and onetime can be removed (also because it can change once you "Update branch in the PR)
source/features/restore-file.tsx
Outdated
| // Only allow one click | ||
| if (filesRestored.has(menuItem)) { | ||
| return; | ||
| } | ||
|
|
||
| filesRestored.add(menuItem); |
There was a problem hiding this comment.
We can drop this too. It's part of the old logic that kept the menu open to display the status. Thanks to showToast, double-clicks are impossible now
|
Oh I'm sorry I couldn't make the changes quickly, I just saw the review comments :( |
Related Issues
Closes #6155.
Test URLs
https://github.com/gamemaker1/test/pull/1/files