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.

Sign up for GitHub

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

chore(quickie): delete quickie for gh #1043

Merged
merged 2 commits into from
Dec 6, 2023
Merged

chore(quickie): delete quickie for gh #1043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7ce64f
chore(quickie): delete quickie for gh
kishore03109 Dec 3, 2023
a875e6f
Untitled commit
kishore03109 Dec 3, 2023
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
3 changes: 0 additions & 3 deletions src/integration/Notifications.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
import { getAuthorizationMiddleware } from "@root/middleware"
import { NotificationsRouter as _NotificationsRouter } from "@root/routes/v2/authenticated/notifications"
import GitFileCommitService from "@root/services/db/GitFileCommitService"
import GitHubCommitService from "@root/services/db/GithubCommitService"
import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService"
import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService"
import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService"
Expand Down Expand Up @@ -71,12 +70,10 @@ const MOCK_SITE_MEMBER_ID = "1"

const gitFileSystemService = new GitFileSystemService(simpleGit())
const gitFileCommitService = new GitFileCommitService(gitFileSystemService)
const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance)
const gitHubService = new RepoService({
isomerRepoAxiosInstance,
gitFileSystemService,
gitFileCommitService,
gitHubCommitService,
})
const identityAuthService = getIdentityAuthService(gitHubService)
const usersService = getUsersService(sequelize)
Expand Down
4 changes: 1 addition & 3 deletions src/integration/Privatisation.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import { AuthorizationMiddleware } from "@root/middleware/authorization"
import { SettingsRouter as _SettingsRouter } from "@root/routes/v2/authenticatedSites/settings"
import { SettingsService } from "@root/services/configServices/SettingsService"
import GitFileCommitService from "@root/services/db/GitFileCommitService"
import GitHubCommitService from "@root/services/db/GithubCommitService"
import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService"
import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService"
import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService"
Expand Down Expand Up @@ -100,12 +99,11 @@ jest.mock("../services/identity/DeploymentClient", () =>
)
const gitFileSystemService = new GitFileSystemService(simpleGit())
const gitFileCommitService = new GitFileCommitService(gitFileSystemService)
const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance)

const gitHubService = new RepoService({
isomerRepoAxiosInstance,
gitFileSystemService,
gitFileCommitService,
gitHubCommitService,
})
const configYmlService = new ConfigYmlService({ gitHubService })
const usersService = getUsersService(sequelize)
Expand Down
4 changes: 1 addition & 3 deletions src/integration/Reviews.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ import {
} from "@fixtures/users"
import { ReviewRequestStatus } from "@root/constants"
import GitFileCommitService from "@root/services/db/GitFileCommitService"
import GitHubCommitService from "@root/services/db/GithubCommitService"
import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService"
import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService"
import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService"
Expand Down Expand Up @@ -105,12 +104,11 @@ import { sequelize } from "@tests/database"

const gitFileSystemService = new GitFileSystemService(simpleGit())
const gitFileCommitService = new GitFileCommitService(gitFileSystemService)
const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance)

const gitHubService = new RepoService({
isomerRepoAxiosInstance,
gitFileSystemService,
gitFileCommitService,
gitHubCommitService,
})
const configYmlService = new ConfigYmlService({ gitHubService })
const usersService = getUsersService(sequelize)
Expand Down
4 changes: 1 addition & 3 deletions src/integration/Sites.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { statsMiddleware } from "@root/middleware/stats"
import { SitesRouter as _SitesRouter } from "@root/routes/v2/authenticated/sites"
import { isomerRepoAxiosInstance } from "@root/services/api/AxiosInstance"
import GitFileCommitService from "@root/services/db/GitFileCommitService"
import GitHubCommitService from "@root/services/db/GithubCommitService"
import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService"
import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService"
import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService"
Expand Down Expand Up @@ -67,13 +66,12 @@ const mockPermissions = { push: true }
const mockPrivate = true

const gitFileSystemService = new GitFileSystemService(simpleGit())
const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance)

const gitFileCommitService = new GitFileCommitService(gitFileSystemService)
const gitHubService = new RepoService({
isomerRepoAxiosInstance,
gitFileSystemService,
gitFileCommitService,
gitHubCommitService,
})
const configYmlService = new ConfigYmlService({ gitHubService })
const usersService = getUsersService(sequelize)
Expand Down
3 changes: 0 additions & 3 deletions src/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ import { SgidAuthRouter } from "./routes/v2/sgidAuth"
import RepoManagementService from "./services/admin/RepoManagementService"
import GitFileCommitService from "./services/db/GitFileCommitService"
import GitFileSystemService from "./services/db/GitFileSystemService"
import GitHubCommitService from "./services/db/GithubCommitService"
import RepoService from "./services/db/RepoService"
import { PageService } from "./services/fileServices/MdPageServices/PageService"
import { ConfigService } from "./services/fileServices/YmlFileServices/ConfigService"
Expand Down Expand Up @@ -170,14 +169,12 @@ const simpleGitInstance = new simpleGit({
})

const gitFileSystemService = new GitFileSystemService(simpleGitInstance)
const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance)
const gitFileCommitService = new GitFileCommitService(gitFileSystemService)

const gitHubService = new RepoService({
isomerRepoAxiosInstance,
gitFileSystemService,
gitFileCommitService,
gitHubCommitService,
})

const repoManagementService = new RepoManagementService({
Expand Down
32 changes: 11 additions & 21 deletions src/services/db/GitHubService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,11 @@ export default class GitHubService {
fileName,
directoryName,
isMedia = false,
branchName = STAGING_BRANCH,
}: {
content: string
fileName: string
directoryName: string
isMedia: boolean
branchName: string
}
) {
const { accessToken, siteName, isomerUserId: userId } = sessionData
Expand Down Expand Up @@ -185,7 +183,7 @@ export default class GitHubService {
const params = {
message,
content: encodedContent,
branch: branchName,
branch: STAGING_BRANCH,
}

const resp = await this.axiosInstance.put(endpoint, params, {
Expand All @@ -209,18 +207,14 @@ export default class GitHubService {

async read(
sessionData: UserWithSiteSessionData,
{
fileName,
directoryName,
branchName = STAGING_BRANCH,
}: { fileName: any; directoryName: any; branchName?: string }
{ fileName, directoryName }: { fileName: any; directoryName: any }
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
) {
const { accessToken } = sessionData
const { siteName } = sessionData
const endpoint = this.getFilePath({ siteName, fileName, directoryName })

const params = {
ref: branchName,
ref: STAGING_BRANCH,
}

const resp = await this.axiosInstance.get(endpoint, {
Expand All @@ -241,10 +235,7 @@ export default class GitHubService {

async readMedia(
sessionData: UserWithSiteSessionData,
{
fileSha,
branchName = STAGING_BRANCH,
}: { fileSha: any; branchName?: string }
{ fileSha }: { fileSha: any; branchName?: string }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: wrong typing - remove branchName! could we also stricten fileSha to string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, removed

) {
/**
* Files that are bigger than 1 MB needs to be retrieved
Expand All @@ -254,7 +245,7 @@ export default class GitHubService {
const { accessToken } = sessionData
const { siteName } = sessionData
const params = {
ref: branchName,
ref: STAGING_BRANCH,
}

const blobEndpoint = this.getBlobPath({ siteName, fileSha })
Expand Down Expand Up @@ -379,24 +370,21 @@ export default class GitHubService {
sha,
fileName,
directoryName,
branchName = STAGING_BRANCH,
}: {
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
sha: string
fileName: string
directoryName: string
branchName?: string
}
) {
const { accessToken, siteName, isomerUserId: userId } = sessionData
try {
const endpoint = this.getFilePath({ siteName, fileName, directoryName })

let fileSha = sha
if (!sha || branchName === STAGING_LITE_BRANCH) {
if (!sha) {
const { sha: retrievedSha } = await this.read(sessionData, {
fileName,
directoryName,
branchName,
})
fileSha = retrievedSha
}
Expand All @@ -408,7 +396,7 @@ export default class GitHubService {
})
const params = {
message,
branch: branchName,
branch: STAGING_BRANCH,
sha: fileSha,
}

Expand Down Expand Up @@ -767,11 +755,13 @@ export default class GitHubService {

async updateRepoState(
sessionData: UserWithSiteSessionData,
{ commitSha }: { commitSha: string }
{ commitSha, branchName }: { commitSha: any; branchName?: string }
) {
const { accessToken } = sessionData
const { siteName } = sessionData
const refEndpoint = `${siteName}/git/refs/heads/${STAGING_BRANCH}`
const refEndpoint = `${siteName}/git/refs/heads/${
branchName || STAGING_BRANCH
}`
Comment on lines +762 to +764
Copy link
Contributor

Choose a reason for hiding this comment

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

i might be lacking context here but it seems abit weird to me to see branchName allowed as an argument here when we're explicitly removing it elsewhere (cos we know the branch should be STAGING_BRANCH) - is this because there are some call sites that need the branch to be different? (a merge to master perhaps)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the case! Specifically we have a RepoManagementService that calls on this branch name, and thus this API should be preserved

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for checking - seems like we (isomer admins) use it to reset branches so it should still be staging. that being said, it's ok to leave as-is cos this is hard to tell for sure + the added flexibility of resetting a diff branch might be useful for staging/staging-lite/master

const headers = {
Authorization: `token ${accessToken}`,
}
Expand Down
Loading
Loading