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

fix(media): change media sorting to addedTime descending #1019

Merged
merged 2 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/services/db/GitFileSystemService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@ export default class GitFileSystemService {
sha,
path,
size: type === "dir" ? 0 : stats.size,
addedTime: stats.birthtimeMs,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we not need this for github?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, why not use stats.mtimeMs (last modified time - see here)

}

return okAsync(result)
Expand Down
13 changes: 8 additions & 5 deletions src/services/db/RepoService.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { GrowthBook } from "@growthbook/growthbook"
import { AxiosCacheInstance } from "axios-cache-interceptor"
import _ from "lodash"

Expand All @@ -10,7 +9,6 @@ import GithubSessionData from "@root/classes/GithubSessionData"
import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData"
import { FEATURE_FLAGS, STAGING_BRANCH } from "@root/constants"
import { GitHubCommitData } from "@root/types/commitData"
import { FeatureFlags } from "@root/types/featureFlags"
import type {
GitCommitResult,
GitDirectoryItem,
Expand Down Expand Up @@ -43,7 +41,12 @@ const getPaginatedDirectoryContents = (
(item) => item.type === "file" && item.name !== PLACEHOLDER_FILE_NAME
)
const paginatedFiles = _(files)
.sortBy(["name"])
// Note: We are sorting by name here to maintain compatibility for
// GitHub-login users, since it is very expensive to get the addedTime for
// each file from the GitHub API. The files will be sorted by addedTime in
// milliseconds for GGS users, so they will never see the alphabetical
// sorting.
.orderBy(["addedTime", "name"], ["desc", "asc"])
.drop(page * limit)
.take(limit)
.value()
Expand Down Expand Up @@ -588,7 +591,7 @@ export default class RepoService extends GitHubService {
{ gitTree, message }: any,
isStaging: boolean
): Promise<any> {
return await super.updateTree(
return super.updateTree(
sessionData,
githubSessionData,
{
Expand Down Expand Up @@ -638,6 +641,6 @@ export default class RepoService extends GitHubService {
sessionData: any,
shouldMakePrivate: any
): Promise<any> {
return await super.changeRepoPrivacy(sessionData, shouldMakePrivate)
return super.changeRepoPrivacy(sessionData, shouldMakePrivate)
}
}
16 changes: 16 additions & 0 deletions src/services/db/__tests__/GitFileSystemService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,27 +90,38 @@ describe("GitFileSystemService", () => {
sha: "fake-dir-hash",
path: "fake-dir",
size: 0,
addedTime: fs.statSync(`${EFS_VOL_PATH_STAGING}/fake-repo/fake-dir`)
.birthtimeMs,
}
const expectedAnotherFakeDir: GitDirectoryItem = {
name: "another-fake-dir",
type: "dir",
sha: "another-fake-dir-hash",
path: "another-fake-dir",
size: 0,
addedTime: fs.statSync(
`${EFS_VOL_PATH_STAGING}/fake-repo/another-fake-dir`
).birthtimeMs,
}
const expectedFakeEmptyDir: GitDirectoryItem = {
name: "fake-empty-dir",
type: "dir",
sha: "fake-empty-dir-hash",
path: "fake-empty-dir",
size: 0,
addedTime: fs.statSync(
`${EFS_VOL_PATH_STAGING}/fake-repo/fake-empty-dir`
).birthtimeMs,
}
const expectedAnotherFakeFile: GitDirectoryItem = {
name: "another-fake-file",
type: "file",
sha: "another-fake-file-hash",
path: "another-fake-file",
size: "Another fake content".length,
addedTime: fs.statSync(
`${EFS_VOL_PATH_STAGING}/fake-repo/another-fake-file`
).birthtimeMs,
}

const result = await GitFileSystemService.listDirectoryContents(
Expand Down Expand Up @@ -150,13 +161,18 @@ describe("GitFileSystemService", () => {
sha: "fake-dir-hash",
path: "fake-dir",
size: 0,
addedTime: fs.statSync(`${EFS_VOL_PATH_STAGING}/fake-repo/fake-dir`)
.birthtimeMs,
}
const expectedAnotherFakeFile: GitDirectoryItem = {
name: "another-fake-file",
type: "file",
sha: "another-fake-file-hash",
path: "another-fake-file",
size: "Another fake content".length,
addedTime: fs.statSync(
`${EFS_VOL_PATH_STAGING}/fake-repo/another-fake-file`
).birthtimeMs,
}

const result = await GitFileSystemService.listDirectoryContents(
Expand Down
6 changes: 6 additions & 0 deletions src/services/db/__tests__/RepoService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,20 +341,23 @@ describe("RepoService", () => {
sha: "test-sha1",
path: "test/fake-file.md",
size: 100,
addedTime: 3,
},
{
name: "another-fake-file.md",
type: "file",
sha: "test-sha2",
path: "another-fake-file.md",
size: 100,
addedTime: 2,
},
{
name: "fake-dir",
type: "dir",
sha: "test-sha3",
path: "fake-dir",
size: 0,
addedTime: 1,
},
]
gbSpy.mockReturnValueOnce(true)
Expand Down Expand Up @@ -387,20 +390,23 @@ describe("RepoService", () => {
sha: "test-sha1",
path: "test/fake-file.md",
size: 100,
addedTime: 3,
},
{
name: "another-fake-file.md",
type: "file",
sha: "test-sha2",
path: "another-fake-file.md",
size: 100,
addedTime: 2,
},
{
name: "fake-dir",
type: "dir",
sha: "test-sha3",
path: "fake-dir",
size: 0,
addedTime: 1,
},
]
const gitHubServiceReadDirectory = jest.spyOn(
Expand Down
1 change: 1 addition & 0 deletions src/types/gitfilesystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ export type GitDirectoryItem = {
sha: string
path: string
size: number
addedTime: number
}
Loading