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

[GH-669]: "Add org selector to create issue modal" thoratvinod:issue-669 #766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
125 changes: 121 additions & 4 deletions server/plugin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ func (p *Plugin) initializeAPI() {
apiRouter.HandleFunc("/labels", p.checkAuth(p.attachUserContext(p.getLabels), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/milestones", p.checkAuth(p.attachUserContext(p.getMilestones), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/assignees", p.checkAuth(p.attachUserContext(p.getAssignees), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/organizations", p.checkAuth(p.attachUserContext(p.getOrganizations), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/repos_by_org", p.checkAuth(p.attachUserContext(p.getReposByOrg), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/repositories", p.checkAuth(p.attachUserContext(p.getRepositories), ResponseTypePlain)).Methods(http.MethodGet)
apiRouter.HandleFunc("/settings", p.checkAuth(p.attachUserContext(p.updateSettings), ResponseTypePlain)).Methods(http.MethodPost)
apiRouter.HandleFunc("/issue", p.checkAuth(p.attachUserContext(p.getIssueByNumber), ResponseTypePlain)).Methods(http.MethodGet)
Expand Down Expand Up @@ -1186,10 +1188,29 @@ func (p *Plugin) getMilestones(c *UserContext, w http.ResponseWriter, r *http.Re
p.writeJSON(w, allMilestones)
}

func getRepositoryList(c context.Context, userName string, githubClient *github.Client, opt github.ListOptions) ([]*github.Repository, error) {
func getOrganizationList(c context.Context, userName string, githubClient *github.Client, opt github.ListOptions) ([]*github.Organization, error) {
var allOrgs []*github.Organization
for {
orgs, resp, err := githubClient.Organizations.List(c, userName, &opt)
if err != nil {
return nil, err
}

allOrgs = append(allOrgs, orgs...)
if resp.NextPage == 0 {
break
}

opt.Page = resp.NextPage
}

return allOrgs, nil
}

func getRepositoryList(c context.Context, userName string, githubClient *github.Client, opt github.RepositoryListOptions) ([]*github.Repository, error) {
var allRepos []*github.Repository
for {
repos, resp, err := githubClient.Repositories.List(c, userName, &github.RepositoryListOptions{ListOptions: opt})
repos, resp, err := githubClient.Repositories.List(c, userName, &opt)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1223,6 +1244,102 @@ func getRepositoryListByOrg(c context.Context, org string, githubClient *github.
return allRepos, http.StatusOK, nil
}

func (p *Plugin) getOrganizations(c *UserContext, w http.ResponseWriter, r *http.Request) {
var allOrgs []*github.Organization
org := p.getConfiguration().GitHubOrg

if org == "" {
includeLoggedInUser := r.URL.Query().Get("includeLoggedInUser")
if includeLoggedInUser == "true" {
allOrgs = append(allOrgs, &github.Organization{Login: &c.GHInfo.GitHubUsername})
}
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)
orgList, err := getOrganizationList(c.Ctx, "", githubClient, github.ListOptions{PerPage: 50})
if err != nil {
c.Log.WithError(err).Warnf("Failed to list organizations")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch organizations", StatusCode: http.StatusInternalServerError})
return
}
allOrgs = append(allOrgs, orgList...)
} else {
allOrgs = append(allOrgs, &github.Organization{Login: &org})
}
// Only send required organizations to the client
type OrganizationResponse struct {
Login string `json:"login,omitempty"`
}

resp := make([]*OrganizationResponse, len(allOrgs))
for i, r := range allOrgs {
ayusht2810 marked this conversation as resolved.
Show resolved Hide resolved
resp[i] = &OrganizationResponse{
Login: r.GetLogin(),
}
}

p.writeJSON(w, resp)
}

func (p *Plugin) getReposByOrg(c *UserContext, w http.ResponseWriter, r *http.Request) {
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)

opt := github.ListOptions{PerPage: 50}

org := r.URL.Query().Get("organization")

if org == "" {
c.Log.Warnf("Organization query param is empty")
p.writeAPIError(w, &APIErrorResponse{Message: "Organization query is empty, must include organization name ", StatusCode: http.StatusBadRequest})
return
}

var allRepos []*github.Repository
var err error
var statusCode int

// If an organization is the username of an authenticated user then return repos where the authenticated user is the owner
if org == c.GHInfo.GitHubUsername {
allRepos, err = getRepositoryList(c.Ctx, "", githubClient, github.RepositoryListOptions{ListOptions: opt, Affiliation: "owner"})
if err != nil {
c.Log.WithError(err).Warnf("Failed to list repositories")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
return
}
} else {
allRepos, statusCode, err = getRepositoryListByOrg(c.Ctx, org, githubClient, opt)
if err != nil {
if statusCode == http.StatusNotFound {
allRepos, err = getRepositoryList(c.Ctx, org, githubClient, github.RepositoryListOptions{ListOptions: opt})
ayusht2810 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
c.Log.WithError(err).Warnf("Failed to list repositories")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
return
}
} else {
c.Log.WithError(err).Warnf("Failed to list repositories")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: statusCode})
return
}
}
}
// Only send repositories which are part of the requested organization
type RepositoryResponse struct {
Name string `json:"name,omitempty"`
FullName string `json:"full_name,omitempty"`
Permissions map[string]bool `json:"permissions,omitempty"`
}

resp := make([]*RepositoryResponse, len(allRepos))
for i, r := range allRepos {
resp[i] = &RepositoryResponse{
Name: r.GetName(),
FullName: r.GetFullName(),
Permissions: r.GetPermissions(),
}
}

p.writeJSON(w, resp)
}

func (p *Plugin) getRepositories(c *UserContext, w http.ResponseWriter, r *http.Request) {
githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)

Expand All @@ -1234,7 +1351,7 @@ func (p *Plugin) getRepositories(c *UserContext, w http.ResponseWriter, r *http.
opt := github.ListOptions{PerPage: 50}

if org == "" {
allRepos, err = getRepositoryList(c.Ctx, "", githubClient, opt)
allRepos, err = getRepositoryList(c.Ctx, "", githubClient, github.RepositoryListOptions{ListOptions: opt})
if err != nil {
c.Log.WithError(err).Warnf("Failed to list repositories")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
Expand All @@ -1244,7 +1361,7 @@ func (p *Plugin) getRepositories(c *UserContext, w http.ResponseWriter, r *http.
allRepos, statusCode, err = getRepositoryListByOrg(c.Ctx, org, githubClient, opt)
if err != nil {
if statusCode == http.StatusNotFound {
allRepos, err = getRepositoryList(c.Ctx, org, githubClient, opt)
allRepos, err = getRepositoryList(c.Ctx, org, githubClient, github.RepositoryListOptions{ListOptions: opt})
if err != nil {
c.Log.WithError(err).Warnf("Failed to list repositories")
p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
Expand Down
2 changes: 2 additions & 0 deletions webapp/src/action_types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const {id: pluginId} = manifest;

export default {
RECEIVED_REPOSITORIES: pluginId + '_received_repositories',
RECEIVED_ORGANIZATIONS: pluginId + '_received_organizations',
RECEIVED_REPOSITORIES_BY_ORGANIZATION: pluginId + '_received_repositories_by_organization',
RECEIVED_REVIEWS_DETAILS: pluginId + '_received_reviews_details',
RECEIVED_YOUR_PRS_DETAILS: pluginId + '_received_your_prs_details',
RECEIVED_SIDEBAR_CONTENT: pluginId + '_received_sidebar_content',
Expand Down
46 changes: 46 additions & 0 deletions webapp/src/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,52 @@ export function getReviewsDetails(prList: PrsDetailsData[]) {
};
}

export function getOrgs() {
return async (dispatch: DispatchFunc) => {
let data;
try {
data = await Client.getOrganizations();
} catch (error) {
return {error: data};
}

const connected = await checkAndHandleNotConnected(data)(dispatch);
if (!connected) {
return {error: data};
}

dispatch({
type: ActionTypes.RECEIVED_ORGANIZATIONS,
data,
});

return {data};
};
}

export function getReposByOrg(organization: string) {
return async (dispatch: DispatchFunc) => {
let data;
try {
data = await Client.getRepositoriesByOrganization(organization);
} catch (error) {
return {error: data};
}

const connected = await checkAndHandleNotConnected(data)(dispatch);
if (!connected) {
return {error: data};
}

dispatch({
type: ActionTypes.RECEIVED_REPOSITORIES_BY_ORGANIZATION,
data,
});

return {data};
};
}

export function getRepos() {
return async (dispatch: DispatchFunc) => {
let data;
Expand Down
8 changes: 8 additions & 0 deletions webapp/src/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,18 @@ export default class Client {
return this.doPost(`${this.url}/user`, {user_id: userID});
}

getOrganizations = async () => {
return this.doGet(`${this.url}/organizations?includeLoggedInUser=true`);
}

getRepositories = async () => {
return this.doGet(`${this.url}/repositories`);
}

getRepositoriesByOrganization = async (organization) => {
return this.doGet(`${this.url}/repos_by_org?organization=${organization}`);
}

getLabels = async (repo) => {
return this.doGet(`${this.url}/labels?repo=${repo}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,21 @@ import ReactSelectSetting from 'components/react_select_setting';
const initialState = {
invalid: false,
error: null,
org: '',
};

export default class GithubRepoSelector extends PureComponent {
static propTypes = {
yourRepos: PropTypes.array.isRequired,
yourOrgs: PropTypes.array.isRequired,
yourReposByOrg: PropTypes.array,
theme: PropTypes.object.isRequired,
onChange: PropTypes.func.isRequired,
value: PropTypes.string,
addValidate: PropTypes.func,
removeValidate: PropTypes.func,
actions: PropTypes.shape({
getRepos: PropTypes.func.isRequired,
getOrgs: PropTypes.func.isRequired,
getReposByOrg: PropTypes.func.isRequired,
}).isRequired,
};

Expand All @@ -30,25 +33,76 @@ export default class GithubRepoSelector extends PureComponent {
}

componentDidMount() {
this.props.actions.getRepos();
this.props.actions.getOrgs();
}

onChange = (_, name) => {
const repo = this.props.yourRepos.find((r) => r.full_name === name);
componentDidUpdate(prevProps) {
if (prevProps.yourOrgs !== this.props.yourOrgs) {
if (this.props.yourOrgs.length) {
this.onChangeForOrg(0, this.props.yourOrgs[0].login);
}
}
}

onChangeForOrg = (_, org) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we remove the first parameter here, if it is not used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could probably change it... the signature here is the same before changes:

onChange = (_, name) => {
const repo = this.props.yourRepos.find((r) => r.full_name === name);
this.props.onChange({name, permissions: repo.permissions});
}

Not sure exactly why it was written this way but I'll remove placeholder from here and the other onChange signature mention in the another suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just tested removing the first parameter and it seems like it 's vital to the functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please tell us what errors or issues you had after making the changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The repository now displays what I think are the local files on the server after removing the first parameter as shown:

Suggested change
onChangeForOrg = (_, org) => {
onChangeForOrg = (org) => {

image

if (this.state.org !== org) {
this.setState({org});
this.props.actions.getReposByOrg(org);
this.props.onChange(null);
}
}

onChangeForRepo = (_, name) => {
cyrusjc marked this conversation as resolved.
Show resolved Hide resolved
const repo = this.props.yourReposByOrg.find((r) => r.full_name === name);
this.props.onChange({name, permissions: repo.permissions});
}

render() {
const repoOptions = this.props.yourRepos.map((item) => ({value: item.full_name, label: item.full_name}));
const orgOptions = this.props.yourOrgs.map((item) => ({value: item.login, label: item.login}));
const repoOptions = this.props.yourReposByOrg.map((item) => ({value: item.full_name, label: item.name}));

let orgSelector = null;
let helperTextForRepoSelector = 'Returns GitHub repositories connected to the user account';

// If there are no organizations for authenticated user, then don't show organization selector
if (orgOptions.length > 1) {
orgSelector = (
<>
<ReactSelectSetting
name={'org'}
label={'Organization'}
limitOptions={true}
required={true}
onChange={this.onChangeForOrg}
options={orgOptions}
isMulti={false}
key={'org'}
theme={this.props.theme}
addValidate={this.props.addValidate}
formatGroupLabel='user repositories'
removeValidate={this.props.removeValidate}
value={orgOptions.find((option) => option.value === this.state.org)}
/>
<div
className='help-text'
style={{marginBottom: '15px'}}
>
{'Returns GitHub organizations connected to the user account'}
</div>
</>
);
helperTextForRepoSelector = 'Returns GitHub repositories under selected organizations';
}

return (
<div className={'form-group margin-bottom x3'}>
{orgSelector}
<ReactSelectSetting
name={'repo'}
label={'Repository'}
limitOptions={true}
required={true}
onChange={this.onChange}
onChange={this.onChangeForRepo}
options={repoOptions}
isMulti={false}
key={'repo'}
Expand All @@ -58,7 +112,7 @@ export default class GithubRepoSelector extends PureComponent {
value={repoOptions.find((option) => option.value === this.props.value)}
/>
<div className={'help-text'}>
{'Returns GitHub repositories connected to the user account'} <br/>
{helperTextForRepoSelector}
</div>
</div>
);
Expand Down
16 changes: 11 additions & 5 deletions webapp/src/components/github_repo_selector/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,27 @@ import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';

import manifest from 'manifest';
import {getRepos} from '../../actions';

import {getReposByOrg, getOrgs} from '../../actions';

import GithubRepoSelector from './github_repo_selector.jsx';

function mapStateToProps(state) {
return {
yourRepos: state[`plugins-${manifest.id}`].yourRepos,
yourOrgs: state[`plugins-${manifest.id}`].yourOrgs,
yourReposByOrg: state[`plugins-${manifest.id}`].yourReposByOrg,
};
}

function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators({
getRepos,
}, dispatch),
actions: bindActionCreators(
{
getOrgs,
getReposByOrg,
},
dispatch,
),
};
}

Expand Down
Loading
Loading