Skip to content

Commit 9c71def

Browse files
deniakdeniak
authored
allow admin to update a repo owner/name after it's transferred (#277)
* allow admin to update a repo owner/name after it's transfered * update only attributes provided in the request * check if the repo exists in the edit mode * fix tests * rebuild app.js
1 parent 361a6e6 commit 9c71def

File tree

4 files changed

+76
-36
lines changed

4 files changed

+76
-36
lines changed

‎application/repo-manager.jsx‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ export default class RepoNew extends React.Component {
5151
fetch(pp + "api/repos")
5252
.then(utils.jsonHandler)
5353
.then(repos => {
54-
this.updateState({initGroups: repos.find(r => r.owner === st.org && r.name === st.repo).groups.map(g => g.w3cid)});
54+
const repo = repos.find(r => r.owner === st.org && r.name === st.repo);
55+
if (!repo) {
56+
MessageActions.error("Repository not found");
57+
return this.updateState({status: "ready"});
58+
}
59+
this.updateState({initGroups: repo.groups.map(g => g.w3cid)});
5560
});
5661
}
5762
fetch(pp + "api/my/last-added-repo", { credentials: "include" })
@@ -98,16 +103,17 @@ export default class RepoNew extends React.Component {
98103
if (!Object.keys(this.state.orgRepos).length) return;
99104
let org = utils.val(this.refs.org);
100105
switch(this.state.mode) {
101-
case "new":
102-
if (this.state.orgRepos[org].indexOf(ev.target.value) !== -1) {
103-
return ev.target.setCustomValidity("Can't create a repo with that name - already exists");
104-
}
105-
break;
106-
case "import":
107-
if (this.state.orgRepos[org].indexOf(ev.target.value) === -1) {
108-
return ev.target.setCustomValidity("Can't import a repo with that name - does not exist");
109-
}
110-
break;
106+
case "new":
107+
if (this.state.orgRepos[org].indexOf(ev.target.value) !== -1) {
108+
return ev.target.setCustomValidity("Can't create a repo with that name - already exists");
109+
}
110+
break;
111+
case "import":
112+
case "edit":
113+
if (this.state.orgRepos[org].indexOf(ev.target.value) === -1) {
114+
return ev.target.setCustomValidity(`Can't ${this.state.mode} a repo with that name - does not exist`);
115+
}
116+
break;
111117
}
112118
ev.target.setCustomValidity("");
113119
}
@@ -149,7 +155,7 @@ export default class RepoNew extends React.Component {
149155
apiPath = "api/create-repo";
150156
break;
151157
case "edit":
152-
apiPath = "api/repos/" + org + "/" + repo + "/edit";
158+
apiPath = "api/repos/" + this.props.params.owner + "/" + this.props.params.shortname + "/edit";
153159
break;
154160
case "import":
155161
apiPath = "api/import-repo";
@@ -220,8 +226,7 @@ export default class RepoNew extends React.Component {
220226

221227
render () {
222228
let st = this.state
223-
, results = ""
224-
, readonly = st.mode === "edit";
229+
, results = "";
225230
let org = st.org || (st.orgs ? st.orgs[0] : null);
226231
let repos = org && Object.keys(st.orgRepos).length ? st.orgRepos[org] : [];
227232
let selectedGroupType = st.repoGroups.length ? st.groups.filter(g => g.w3cid == st.repoGroups[0])[0].groupType : null;
@@ -261,12 +266,12 @@ export default class RepoNew extends React.Component {
261266
<form onSubmit={this.onSubmit.bind(this)} ref="form">
262267
<div className="formline">
263268
<label htmlFor="repo">pick organisation or account, and repository name</label>
264-
<select disabled={readonly} ref="org" defaultValue={st.org ? st.org : st.lastAddedRepo.org} required onChange={this.updateOrg.bind(this)}>
269+
<select ref="org" defaultValue={st.org ? st.org : st.lastAddedRepo.org} required onChange={this.updateOrg.bind(this)}>
265270
{st.orgs.map((org) => { return <option value={org} key={org}>{org}</option>; })}
266271
</select>
267272
{" / "}
268-
<input readOnly={readonly} type="text" ref="repo" id="repo" defaultValue={st.repo} required list="repos" onChange={this.onRepoNameChange.bind(this)}/>
269-
{(st.mode === "import") ?
273+
<input type="text" ref="repo" id="repo" defaultValue={st.repo} required list="repos" onChange={this.onRepoNameChange.bind(this)}/>
274+
{(st.mode === "import" || st.mode === "edit") ?
270275
<datalist id="repos">
271276
{repos.map(repo => {
272277
return <option key={org +"/" + repo}>{repo}</option>;
@@ -336,7 +341,7 @@ export default class RepoNew extends React.Component {
336341
return <div className="primary-app">
337342
<h2>Update Repository Data</h2>
338343
<p>
339-
Use the form below to update the group(s) to which an existing managed repository is associated.
344+
Use the form below to update the owner/name of the repository and/or the group(s) to which this repository is associated with.
340345
</p>
341346
{content}
342347
{results}

‎public/js/app.js‎

Lines changed: 15 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎server.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,26 @@ router.post("/api/revalidate", bp.json(), async (req, res) => {
361361
// GITHUB APIs
362362
router.post("/api/create-repo", ensureAPIAuth, bp.json(), loadGH, makeCreateOrImportRepo("create"));
363363
router.post("/api/import-repo", ensureAPIAuth, bp.json(), loadGH, makeCreateOrImportRepo("import"));
364-
router.post("/api/repos/:owner/:shortname/edit", ensureAdmin, bp.json(), function(req, res) {
365-
store.updateRepo(req.params.owner + "/" + req.params.shortname, {groups: req.body.groups}, function(err, data) {
364+
router.post("/api/repos/:owner/:shortname/edit", ensureAdmin, bp.json(), async function(req, res) {
365+
const owner = req.body.org;
366+
const repo = req.body.repo;
367+
const groups = req.body.groups || [];
368+
if (owner || repo) {
369+
await doAsync(store).updateSecret(`${req.params.owner}/${req.params.shortname}`, {repo: `${owner}/${repo}`});
370+
}
371+
372+
store.updateRepo(`${req.params.owner}/${req.params.shortname}`, {
373+
...(owner && {owner}),
374+
...(repo && {name: repo}),
375+
...((owner || repo) && {fullName: `${owner}/${repo}`}),
376+
...(groups.length > 0 && {groups})
377+
}, function(err, data) {
366378
if (err) return makeRes(res)(err);
367-
data.actions = ["Moved repository to groups with id " + req.body.groups.join(", ")];
379+
data.actions = [
380+
...(groups.length > 0 ? [`moved repository to groups with id ${groups.join(", ")}`] : []),
381+
...(owner ? [`changed repository owner to ${owner}`] : []),
382+
...(repo ? [`changed repository name to ${repo}`] : [])
383+
];
368384
makeRes(res)(null, data);
369385
});
370386
});

‎test/server-spec.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,11 @@ function RepoMock(_id, _name, _owner, _files, _hooks) {
122122
}
123123

124124
var testNewRepo = new RepoMock(123, "newrepo", "acme", [], []);
125-
var testExistingRepo = new RepoMock(456, "existingrepo","acme", ["README.md"], []);
126-
var testCGRepo = new RepoMock(789, "cgrepo","acme", ["README.md", "index.html"], []);
125+
var testExistingRepo = new RepoMock(456, "existingrepo", "acme", ["README.md"], []);
126+
var testCGRepo = new RepoMock(789, "cgrepo", "acme", ["README.md", "index.html"], []);
127127
const renamedRepo = "existingrepo-bis";
128+
const repoNewOwner = "acme2";
129+
const repoNewName = "newrepo2";
128130

129131
var testPR = {
130132
repository: testExistingRepo.toGH(),
@@ -624,7 +626,7 @@ describe('Server manages requests from advanced privileged users in a set up rep
624626
cleanStore("deleteUser")(testUser.username),
625627
cleanStore("deleteGroup")("" + w3cGroup.id),
626628
cleanStore("deleteGroup")("" + w3cGroup3.id),
627-
cleanStore("deleteRepo")(testNewRepo.full_name),
629+
cleanStore("deleteRepo")(`${repoNewOwner}/${repoNewName}`),
628630
cleanStore("deleteRepo")(`${testExistingRepo.owner}/${renamedRepo}`),
629631
cleanStore("deleteRepo")(testCGRepo.full_name),
630632
cleanStore("deleteToken")(testOrg.login),
@@ -919,6 +921,20 @@ describe('Server manages requests from advanced privileged users in a set up rep
919921
});
920922
});
921923

924+
it('allows admins to update a repository owner and/or name', function testUpdateRepoOwnerName(done) {
925+
authAgent
926+
.post('/api/repos/' + testNewRepo.full_name + '/edit')
927+
.send({org: repoNewOwner, repo: repoNewName})
928+
.expect(200)
929+
.end(function(err, res) {
930+
req.get('/api/repos')
931+
.expect(200, function(err, res) {
932+
expect(res.body.filter(g => g.fullName === `${repoNewOwner}/${repoNewName}`)[0].groups[0].w3cid).to.be("" + w3cGroup2.id);
933+
done();
934+
});
935+
});
936+
});
937+
922938
it('allows logged-in users to revalidate a PR', function testRevalidate(done) {
923939
mockPRStatus(testPR, 'pending', /.*/);
924940
nock('https://api.github.com')

0 commit comments

Comments
 (0)