Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git storage: handle relocated files #4307

Merged
merged 1 commit into from
Sep 1, 2021
Merged

Conversation

EricFromCanada
Copy link
Contributor

Currently, the git storage model does not handle files being renamed. This PR implements this by noticing when a filename in a diff contains => and relocating the corresponding page.

@@ -704,9 +712,11 @@ module.exports = class Page extends Model {
const destinationHash = pageHelper.generateHash({ path: opts.destinationPath, locale: opts.destinationLocale, privateNS: opts.isPrivate ? 'TODO' : '' })

// -> Move page
const destinationTitle = (page.title === page.path ? opts.destinationPath : page.title)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the page has a title that was auto-generated from its filename, this updates its title to match the new filename.

@@ -174,11 +177,25 @@ module.exports = {
async processFiles(files, user) {
for (const item of files) {
const contentType = pageHelper.getContentType(item.relPath)
const fileExists = await fs.pathExists(item.file)
const fileExists = await fs.pathExists(item.file.path)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for fileExists always being false.

Comment on lines +227 to +242
if (fileExists && ((item.before === item.after) || (item.deletions === 0 && item.insertions === 0))) {
// Asset was renamed by git, so rename in DB
WIKI.logger.info(`(STORAGE/GIT) Asset marked as renamed: from ${item.oldPath} to ${item.relPath}`)

const fileHash = assetHelper.generateHash(item.relPath)
const assetToRename = await WIKI.models.assets.query().findOne({ hash: fileHash })
if (assetToRename) {
await WIKI.models.assets.query().patch({
filename: item.relPath,
hash: fileHash
}).findById(assetToRename.id)
await assetToRename.deleteAssetCache()
} else {
WIKI.logger.info(`(STORAGE/GIT) Asset was not found in the DB, nothing to rename: ${item.relPath}`)
}
continue
Copy link
Contributor Author

@EricFromCanada EricFromCanada Aug 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should theoretically work for assets, but wasn't accessible in my testing because diff.files earlier in the file wasn't being populated when a binary was renamed. (Possibly related: steveukx/git-js#243)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a limitation of git-js, which doesn't notice binaries being renamed: steveukx/git-js#885

@Jeewes
Copy link

Jeewes commented Oct 14, 2021

This sounds really great. Good job and thanks 👍 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants