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

Allow creating an alias for repositories #12000

Merged
merged 8 commits into from
Apr 15, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src/lib/databases/repositories-database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface IDatabaseRepository {
readonly id?: number
readonly gitHubRepositoryID: number | null
readonly path: string
readonly alias: string | null
readonly missing: boolean

/** The last time the stash entries were checked for the repository */
Expand Down
5 changes: 5 additions & 0 deletions app/src/lib/feature-flag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,8 @@ export function enableUpdateFromRosettaToARM64(): boolean {
export function enableSaveDialogOnCloneRepository(): boolean {
return enableBetaFeatures()
}

/** Should we allow setting repository aliases? */
export function enableRepositoryAliases(): boolean {
return enableBetaFeatures()
}
15 changes: 15 additions & 0 deletions app/src/lib/stores/app-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,13 @@ export class AppStore extends TypedBaseStore<IAppState> {
previousRepositoryId: number | null,
currentRepositoryId: number
) {
// No need to update the recent repositories if the selected repository is
// the same as the old one (this could happen when the alias of the selected
// repository is changed).
if (previousRepositoryId === currentRepositoryId) {
return
}
Comment on lines +1454 to +1459
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, creating/changing/removing the alias of the current repo, would move the current repo into the Recents list, which doesn't make sense.


const recentRepositories = getNumberArray(RecentRepositoriesKey).filter(
el => el !== currentRepositoryId && el !== previousRepositoryId
)
Expand Down Expand Up @@ -3408,6 +3415,14 @@ export class AppStore extends TypedBaseStore<IAppState> {
return Promise.resolve()
}

/** This shouldn't be called directly. See `Dispatcher`. */
public async _changeRepositoryAlias(
repository: Repository,
newAlias: string | null
): Promise<void> {
return this.repositoriesStore.updateRepositoryAlias(repository, newAlias)
}

/** This shouldn't be called directly. See `Dispatcher`. */
public async _renameBranch(
repository: Repository,
Expand Down
22 changes: 22 additions & 0 deletions app/src/lib/stores/repositories-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { WorkflowPreferences } from '../../models/workflow-preferences'
import { clearTagsToPush } from './helpers/tags-to-push-storage'
import { IMatchedGitHubRepository } from '../repository-matching'
import { shallowEquals } from '../equality'
import { enableRepositoryAliases } from '../feature-flag'

/** The store for local repositories. */
export class RepositoriesStore extends TypedBaseStore<
Expand Down Expand Up @@ -132,6 +133,7 @@ export class RepositoriesStore extends TypedBaseStore<
? await this.findGitHubRepositoryByID(repo.gitHubRepositoryID)
: await Promise.resolve(null), // Dexie gets confused if we return null
repo.missing,
enableRepositoryAliases() ? repo.alias : null,
Copy link
Member Author

Choose a reason for hiding this comment

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

This prevents the alias from being loaded from persistence, so we can forget about checking the feature flag in many other places 😅

repo.workflowPreferences,
repo.isTutorialRepository
)
Expand Down Expand Up @@ -194,6 +196,7 @@ export class RepositoriesStore extends TypedBaseStore<
return await this.db.repositories.put({
...(existingRepo?.id !== undefined && { id: existingRepo.id }),
path,
alias: null,
gitHubRepositoryID: ghRepo.dbID,
missing: false,
lastStashCheckDate: null,
Expand Down Expand Up @@ -228,6 +231,7 @@ export class RepositoriesStore extends TypedBaseStore<
gitHubRepositoryID: null,
missing: false,
lastStashCheckDate: null,
alias: null,
}
const id = await this.db.repositories.add(dbRepo)
return this.toRepository({ id, ...dbRepo })
Expand Down Expand Up @@ -261,11 +265,27 @@ export class RepositoriesStore extends TypedBaseStore<
repository.id,
repository.gitHubRepository,
missing,
repository.alias,
repository.workflowPreferences,
repository.isTutorialRepository
)
}

/**
* Update the alias for the specified repository.
*
* @param repository The repository to update.
* @param alias The new alias to use.
*/
public async updateRepositoryAlias(
repository: Repository,
alias: string | null
): Promise<void> {
await this.db.repositories.update(repository.id, { alias })

this.emitUpdatedRepositories()
}

/**
* Update the workflow preferences for the specified repository.
*
Expand Down Expand Up @@ -295,6 +315,7 @@ export class RepositoriesStore extends TypedBaseStore<
repository.id,
repository.gitHubRepository,
false,
repository.alias,
repository.workflowPreferences,
repository.isTutorialRepository
)
Expand Down Expand Up @@ -421,6 +442,7 @@ export class RepositoriesStore extends TypedBaseStore<
repo.id,
ghRepo,
repo.missing,
repo.alias,
repo.workflowPreferences,
repo.isTutorialRepository
)
Expand Down
2 changes: 2 additions & 0 deletions app/src/models/popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export enum PopupType {
ConfirmDiscardSelection,
CherryPick,
MoveToApplicationsFolder,
ChangeRepositoryAlias,
}

export type Popup =
Expand Down Expand Up @@ -276,3 +277,4 @@ export type Popup =
sourceBranch: Branch | null
}
| { type: PopupType.MoveToApplicationsFolder }
| { type: PopupType.ChangeRepositoryAlias; repository: Repository }
2 changes: 2 additions & 0 deletions app/src/models/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class Repository {
public readonly id: number,
public readonly gitHubRepository: GitHubRepository | null,
public readonly missing: boolean,
public readonly alias: string | null = null,
public readonly workflowPreferences: WorkflowPreferences = {},
/**
* True if the repository is a tutorial repository created as part of the
Expand All @@ -67,6 +68,7 @@ export class Repository {
this.id,
gitHubRepository?.hash,
this.missing,
this.alias,
this.workflowPreferences.forkContributionTarget,
this.isTutorialRepository
)
Expand Down
13 changes: 12 additions & 1 deletion app/src/ui/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ import { CherryPickCommit } from './drag-elements/cherry-pick-commit'
import classNames from 'classnames'
import { dragAndDropManager } from '../lib/drag-and-drop-manager'
import { MoveToApplicationsFolder } from './move-to-applications-folder'
import { ChangeRepositoryAlias } from './change-repository-alias/change-repository-alias-dialog'

const MinuteInMilliseconds = 1000 * 60
const HourInMilliseconds = MinuteInMilliseconds * 60
Expand Down Expand Up @@ -2048,6 +2049,15 @@ export class App extends React.Component<IAppProps, IAppState> {
/>
)
}
case PopupType.ChangeRepositoryAlias: {
return (
<ChangeRepositoryAlias
dispatcher={this.props.dispatcher}
repository={popup.repository}
onDismissed={onPopupDismissedFn}
/>
)
}
default:
return assertNever(popup, `Unknown popup type: ${popup}`)
}
Expand Down Expand Up @@ -2367,8 +2377,9 @@ export class App extends React.Component<IAppProps, IAppState> {
let icon: OcticonSymbol
let title: string
if (repository) {
const alias = repository instanceof Repository ? repository.alias : null
icon = iconForRepository(repository)
title = repository.name
title = alias ?? repository.name
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the title of the drop down that shows the current repository.

} else if (this.state.repositories.length > 0) {
icon = OcticonSymbol.repo
title = __DARWIN__ ? 'Select a Repository' : 'Select a repository'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import * as React from 'react'

import { Dispatcher } from '../dispatcher'
import { nameOf, Repository } from '../../models/repository'
import { Dialog, DialogContent, DialogFooter } from '../dialog'
import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group'
import { TextBox } from '../lib/text-box'

interface IChangeRepositoryAliasProps {
readonly dispatcher: Dispatcher
readonly onDismissed: () => void
readonly repository: Repository
}

interface IChangeRepositoryAliasState {
readonly newAlias: string
}

export class ChangeRepositoryAlias extends React.Component<
IChangeRepositoryAliasProps,
IChangeRepositoryAliasState
> {
public constructor(props: IChangeRepositoryAliasProps) {
super(props)

this.state = { newAlias: props.repository.alias ?? props.repository.name }
}

public render() {
const repository = this.props.repository
const verb = repository.alias === null ? 'Create' : 'Change'

return (
<Dialog
id="change-repository-alias"
title={
__DARWIN__ ? `${verb} Repository Alias` : `${verb} repository alias`
}
onDismissed={this.props.onDismissed}
onSubmit={this.changeAlias}
>
<DialogContent>
<p>Choose a new alias for the repository "{nameOf(repository)}". </p>
<p>
<TextBox
value={this.state.newAlias}
onValueChanged={this.onNameChanged}
/>
</p>
{repository.gitHubRepository !== null && (
<p className="description">
This will not affect the original repository name on GitHub.
</p>
)}
</DialogContent>

<DialogFooter>
<OkCancelButtonGroup
okButtonText={__DARWIN__ ? `${verb} Alias` : `${verb} alias`}
okButtonDisabled={this.state.newAlias.length === 0}
/>
</DialogFooter>
</Dialog>
)
}

private onNameChanged = (newAlias: string) => {
this.setState({ newAlias })
}

private changeAlias = () => {
this.props.dispatcher.changeRepositoryAlias(
this.props.repository,
this.state.newAlias
)
this.props.onDismissed()
}
}
8 changes: 8 additions & 0 deletions app/src/ui/dispatcher/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,14 @@ export class Dispatcher {
})
}

/** Changes the repository alias to a new name. */
public changeRepositoryAlias(
repository: Repository,
newAlias: string | null
): Promise<void> {
return this.appStore._changeRepositoryAlias(repository, newAlias)
}

/** Rename the branch to a new name. */
public renameBranch(
repository: Repository,
Expand Down
14 changes: 9 additions & 5 deletions app/src/ui/repositories-list/group-repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export function groupRepositories(
const { aheadBehind, changedFilesCount } =
localRepositoryStateLookup.get(r.id) || fallbackValue
const repositoryText =
r instanceof Repository ? [r.name, nameOf(r)] : [r.name]
r instanceof Repository ? [r.alias ?? r.name, nameOf(r)] : [r.name]
Copy link
Member Author

Choose a reason for hiding this comment

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

This basically makes sure the alias is used to look for matches when the user filters the repository list.


return {
text: repositoryText,
Expand Down Expand Up @@ -132,8 +132,10 @@ export function makeRecentRepositoriesGroup(
for (const id of recentRepositories) {
const repository = repositories.find(r => r.id === id)
if (repository !== undefined) {
const existingCount = names.get(repository.name) || 0
names.set(repository.name, existingCount + 1)
const alias = repository instanceof Repository ? repository.alias : null
const name = alias ?? repository.name
const existingCount = names.get(name) || 0
names.set(name, existingCount + 1)
Comment on lines -135 to +138
Copy link
Member Author

Choose a reason for hiding this comment

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

This list of names is used to detect duplicates in the Recents group, to know when we need to disambiguate showing the owner as a prefix (e.g. github/desktop).
In this change I'm just adding the alias instead of the repo name, when available.

}
}

Expand All @@ -147,11 +149,13 @@ export function makeRecentRepositoriesGroup(

const { aheadBehind, changedFilesCount } =
localRepositoryStateLookup.get(id) || fallbackValue
const repositoryAlias =
repository instanceof Repository ? repository.alias : null
const repositoryText =
repository instanceof Repository
? [repository.name, nameOf(repository)]
? [repositoryAlias ?? repository.name, nameOf(repository)]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is also for the repository list filter.

: [repository.name]
const nameCount = names.get(repository.name) || 0
const nameCount = names.get(repositoryAlias ?? repository.name) || 0
Comment on lines -154 to +158
Copy link
Member Author

Choose a reason for hiding this comment

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

This is where we actually check if the repo alias or name is duplicated and need disambiguation.

items.push({
text: repositoryText,
id: id.toString(),
Expand Down
15 changes: 14 additions & 1 deletion app/src/ui/repositories-list/repositories-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from './group-repositories'
import { FilterList, IFilterListGroup } from '../lib/filter-list'
import { IMatches } from '../../lib/fuzzy-find'
import { ILocalRepositoryState } from '../../models/repository'
import { ILocalRepositoryState, Repository } from '../../models/repository'
import { Dispatcher } from '../dispatcher'
import { Button } from '../lib/button'
import { Octicon, OcticonSymbol } from '../octicons'
Expand Down Expand Up @@ -138,6 +138,8 @@ export class RepositoriesList extends React.Component<
onShowRepository={this.props.onShowRepository}
onOpenInShell={this.props.onOpenInShell}
onOpenInExternalEditor={this.props.onOpenInExternalEditor}
onChangeRepositoryAlias={this.onChangeRepositoryAlias}
onRemoveRepositoryAlias={this.onRemoveRepositoryAlias}
externalEditorLabel={this.props.externalEditorLabel}
shellLabel={this.props.shellLabel}
matches={matches}
Expand Down Expand Up @@ -320,4 +322,15 @@ export class RepositoriesList extends React.Component<
private onCreateNewRepository = () => {
this.props.dispatcher.showPopup({ type: PopupType.CreateRepository })
}

private onChangeRepositoryAlias = (repository: Repository) => {
this.props.dispatcher.showPopup({
type: PopupType.ChangeRepositoryAlias,
repository,
})
}

private onRemoveRepositoryAlias = (repository: Repository) => {
this.props.dispatcher.changeRepositoryAlias(repository, null)
}
}