Skip to content

Commit ead0dc7

Browse files
committed
Implemented changes from PR review
- Changed "source" to "content-provider" in both files - In link.html, added detail to regEx for URL patterns - removed circleci folder I thought was needed - In generateBinderUrl, clarified the parameter names to reflect envRepoX and contentRepoX and fixed the generatation of the URL - In binder tab, the environment repo now includes the branch if not specified - added documentation - cleaned up the generation of the URLPath
1 parent 8c99a9d commit ead0dc7

File tree

3 files changed

+106
-97
lines changed

3 files changed

+106
-97
lines changed

.circleci/config.yml

Lines changed: 0 additions & 12 deletions
This file was deleted.

_static/link_gen/link.js

Lines changed: 88 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
// Pure function that generates an nbgitpuller URL
2-
function generateRegularUrl(hubUrl, urlPath, repoUrl, branch, compressed, source) {
2+
function generateRegularUrl(hubUrl, urlPath, repoUrl, branch, compressed, contentProvider) {
33

44
// assume hubUrl is a valid URL
55
var url = new URL(hubUrl);
66

77
url.searchParams.set('repo', repoUrl);
88

99
if(compressed) {
10-
url.searchParams.set('content_provider', source);
10+
url.searchParams.set('content_provider', contentProvider);
1111
}
1212

1313
if (urlPath) {
@@ -16,7 +16,7 @@ function generateRegularUrl(hubUrl, urlPath, repoUrl, branch, compressed, source
1616

1717
if (branch) {
1818
url.searchParams.set('branch', branch);
19-
} else if(source == "git"){
19+
} else if(contentProvider == "git"){
2020
url.searchParams.set('branch', "main");
2121
}
2222

@@ -28,7 +28,7 @@ function generateRegularUrl(hubUrl, urlPath, repoUrl, branch, compressed, source
2828
return url.toString();
2929
}
3030

31-
function generateCanvasUrl(hubUrl, urlPath, repoUrl, branch, compressed, source) {
31+
function generateCanvasUrl(hubUrl, urlPath, repoUrl, branch, compressed, contentProvider) {
3232
// assume hubUrl is a valid URL
3333
var url = new URL(hubUrl);
3434

@@ -37,7 +37,7 @@ function generateCanvasUrl(hubUrl, urlPath, repoUrl, branch, compressed, source)
3737
nextUrlParams.append('repo', repoUrl);
3838

3939
if(compressed) {
40-
nextUrlParams.append('content_provider', source);
40+
nextUrlParams.append('content_provider', contentProvider);
4141
}
4242

4343
if (urlPath) {
@@ -46,7 +46,7 @@ function generateCanvasUrl(hubUrl, urlPath, repoUrl, branch, compressed, source)
4646

4747
if (branch) {
4848
nextUrlParams.append('branch', branch);
49-
} else if(source == "git"){
49+
} else if(contentProvider == "git"){
5050
nextUrlParams.append('branch', "main");
5151
}
5252

@@ -61,33 +61,36 @@ function generateCanvasUrl(hubUrl, urlPath, repoUrl, branch, compressed, source)
6161
return url.toString();
6262
}
6363

64-
function generateBinderUrl(hubUrl, userName, repoName, branch, urlPath,
65-
contentRepoUrl, contentRepoBranch, compressed, source) {
64+
function generateBinderUrl(hubUrl, userName, envRepoName, envGitBranch, urlPath,
65+
contentGitRepoUrl, contentGitRepoBranch, compressed, contentProvider) {
6666

6767
var url = new URL(hubUrl);
6868

6969
var nextUrlParams = new URLSearchParams();
7070

71-
nextUrlParams.append('repo', contentRepoUrl);
71+
nextUrlParams.append('repo', contentGitRepoUrl);
7272

7373
if(compressed) {
74-
nextUrlParams.append('content_provider', source);
74+
nextUrlParams.append('content_provider', contentProvider);
7575
}
7676

7777
if (urlPath) {
7878
nextUrlParams.append('urlpath', urlPath);
7979
}
8080

81-
if (contentRepoBranch) {
82-
nextUrlParams.append('branch', contentRepoBranch);
83-
} else if(source == "git"){
81+
if (contentGitRepoBranch) {
82+
nextUrlParams.append('branch', contentGitRepoBranch);
83+
} else if(contentProvider == "git"){
8484
nextUrlParams.append('branch', "main");
8585
}
8686

87+
if(envGitBranch == ""){
88+
envGitBranch = "main"
89+
}
8790
var nextUrl = 'git-pull?' + nextUrlParams.toString();
8891

8992
var path = '/v2/gh/';
90-
url.pathname = path.concat(userName, "/", repoName, "/", branch);
93+
url.pathname = path.concat(userName, "/", envRepoName, "/", envGitBranch);
9194
url.searchParams.append('urlpath', nextUrl);
9295

9396
return url.toString();
@@ -152,7 +155,7 @@ function changeTab(div) {
152155
env_repo_group.style.display = 'none';
153156
env_repo.disabled = true;
154157
}
155-
displaySource();
158+
displayContentProvider();
156159
}
157160

158161
/**
@@ -161,6 +164,12 @@ function changeTab(div) {
161164
* nbgitpuller needs to redirect users to *inside* the directory it
162165
* just cloned. We copy the logic git itself uses to determine that.
163166
* See https://github.com/git/git/blob/1c52ecf4ba0f4f7af72775695fee653f50737c71/builtin/clone.c#L276
167+
*
168+
* The condition on the first line of this function ensures the user has not included a forward slash at the end of
169+
* the URL. If they do, it is removed so that the rest of the function can split by forward slash and parse the name
170+
* of directory from the URL.
171+
*
172+
* @param {string} gitCloneUrl This is the url to the git repo
164173
*/
165174
function generateCloneDirectoryName(gitCloneUrl) {
166175
if(gitCloneUrl.slice(-1) == "/")
@@ -169,25 +178,38 @@ function generateCloneDirectoryName(gitCloneUrl) {
169178
return lastPart.split(':').slice(-1)[0].replace(/(\.git|\.bundle)?/, '');
170179
}
171180

172-
function handleSource(args){
173-
source = args["source"];
181+
/**
182+
* This takes the values from the UI for content providers and sets three values in a
183+
* json object(contentProviderUrl, branch, and compressed) based on what contentProvider is selected in the UI.
184+
*
185+
* The args contains the contentProvider selected in the UI as well as whatever text(if any)
186+
* is in each of the content provider URL(e.g. driveURL, dropURL) text boxes, as well as the contentGitBranch.
187+
*
188+
* The return value is a a json object containing the branch, which may be an empty string if git is not the content
189+
* provider, the contentProviderURL, and a boolean key, compressed, that indicates if you our notebooks are in a
190+
* compressed archive.
191+
*
192+
* @param {json} args - contains UI element values for content providers(url, git branch,
193+
*/
194+
function configureContentProviderAttrs(args){
195+
contentProvider = args["contentProvider"];
174196
branch = "";
175197
compressed = true;
176-
sourceUrl ="";
177-
if(source == "git"){
178-
sourceUrl = args["contentRepoUrl"];
179-
branch = args["contentRepoBranch"];
198+
contentProviderURL ="";
199+
if(contentProvider == "git"){
200+
contentProviderURL = args["contentGitRepoUrl"];
201+
branch = args["contentGitRepoBranch"];
180202
compressed = false;
181-
} else if(source == "googledrive"){
182-
sourceUrl = args["driveUrl"];
183-
} else if(source == "dropbox"){
184-
sourceUrl = args["dropUrl"];
185-
} else if(source == "generic_web"){
186-
sourceUrl = args["webUrl"];
203+
} else if(contentProvider == "googledrive"){
204+
contentProviderURL = args["driveUrl"];
205+
} else if(contentProvider == "dropbox"){
206+
contentProviderURL = args["dropUrl"];
207+
} else if(contentProvider == "generic_web"){
208+
contentProviderURL = args["webUrl"];
187209
}
188210
return {
189211
"branch": branch,
190-
"sourceUrl": sourceUrl,
212+
"contentProviderURL": contentProviderURL,
191213
"compressed": compressed
192214
}
193215
}
@@ -201,54 +223,51 @@ function displayLink() {
201223
var driveUrl = document.getElementById('drive-url').value;
202224
var dropUrl = document.getElementById('drop-url').value;
203225
var webUrl = document.getElementById('generic-web-url').value;
204-
var envRepoUrl = document.getElementById('env-repo').value;
226+
var envGitRepoUrl = document.getElementById('env-repo').value;
205227
var envGitBranch = document.getElementById('env-branch').value;
206-
var contentRepoUrl = document.getElementById('content-repo').value;
207-
var contentRepoBranch = document.getElementById('content-branch').value;
228+
var contentGitRepoUrl = document.getElementById('content-repo').value;
229+
var contentGitRepoBranch = document.getElementById('content-branch').value;
208230
var filePath = document.getElementById('filepath').value;
209231
var appName = form.querySelector('input[name="app"]:checked').value;
210232
var activeTab = document.querySelector(".nav-link.active").id;
211-
var source = form.querySelector('input[name="source"]:checked').value;
233+
var contentProvider = form.querySelector('input[name="content-provider"]:checked').value;
212234

213235
if (appName === 'custom') {
214236
var urlPath = document.getElementById('urlpath').value;
215237
} else {
216-
var repoName = generateCloneDirectoryName(contentRepoUrl);
217-
if(source !== "git"){
218-
repoName = ""
219-
}
220-
var urlPath;
221-
if (activeTab === "tab-auth-binder") {
222-
var contentRepoName = new URL(contentRepoUrl).pathname.split('/').pop().replace(/\.git$/, '');
223-
urlPath = apps[appName].generateUrlPath(contentRepoName + '/' + filePath);
224-
} else {
225-
urlPath = apps[appName].generateUrlPath(repoName + '/' + filePath);
238+
var envGitRepoName = generateCloneDirectoryName(envGitRepoUrl);
239+
var contentGitRepoName = generateCloneDirectoryName(contentGitRepoUrl);
240+
var partialUrlPath = contentGitRepoName + '/' + filePath;
241+
if(contentProvider !== "git"){
242+
contentGitRepoName = "";
243+
partialUrlPath = filePath;
226244
}
245+
var urlPath = apps[appName].generateUrlPath(partialUrlPath);
227246
}
228247
args = {
229-
"source": source,
230-
"contentRepoUrl": contentRepoUrl,
231-
"contentRepoBranch": contentRepoBranch,
248+
"contentProvider": contentProvider,
249+
"contentGitRepoUrl": contentGitRepoUrl,
250+
"contentGitRepoBranch": contentGitRepoBranch,
232251
"driveUrl": driveUrl,
233252
"dropUrl": dropUrl,
234253
"webUrl": webUrl
235254
}
236-
config = handleSource(args)
255+
config = configureContentProviderAttrs(args)
237256
if (activeTab === "tab-auth-default") {
238257
document.getElementById('default-link').value = generateRegularUrl(
239-
hubUrl, urlPath, config["sourceUrl"], config["branch"], config["compressed"], source
258+
hubUrl, urlPath, config["contentProviderURL"], config["branch"], config["compressed"], contentProvider
240259
);
241260
} else if (activeTab === "tab-auth-canvas"){
242261
document.getElementById('canvas-link').value = generateCanvasUrl(
243-
hubUrl, urlPath, config["sourceUrl"], config["branch"], config["compressed"], source
262+
hubUrl, urlPath, config["contentProviderURL"], config["branch"], config["compressed"], contentProvider
244263
);
245264
} else if (activeTab === "tab-auth-binder"){
246265
// FIXME: userName parsing using new URL(...) assumes a
247266
// HTTP based repoUrl. Does it make sense to create a
248267
// BinderHub link for SSH URLs? Then let's fix this parsing.
249-
var userName = new URL(envRepoUrl).pathname.split('/')[1];
268+
var userName = new URL(envGitRepoUrl).pathname.split('/')[1];
250269
document.getElementById('binder-link').value = generateBinderUrl(
251-
hubUrl, userName, repoName, envGitBranch, urlPath, config["sourceUrl"], config["branch"], config["compressed"], source
270+
hubUrl, userName, envGitRepoName, envGitBranch, urlPath, config["contentProviderURL"], config["branch"], config["compressed"], contentProvider
252271
);
253272
}
254273
} else {
@@ -287,21 +306,23 @@ function hideShowByClassName(cls, hideShow){
287306
});
288307
});
289308
}
290-
291-
292-
function displaySource(){
309+
/**
310+
* Depending on the content provider selected this hides and shows the appropriate divs
311+
*
312+
*/
313+
function displayContentProvider(){
293314
var form = document.getElementById('linkgenerator');
294-
var source = form.querySelector('input[name="source"]:checked').value;
295-
hideShowByClassName(".source", 'none');
296-
297-
if(source == 'git'){
298-
hideShowByClassName(".source-git", '');
299-
} else if(source == 'googledrive'){
300-
hideShowByClassName(".source-googledrive", '');
301-
} else if(source == 'dropbox'){
302-
hideShowByClassName(".source-dropbox", '');
303-
} else if(source =="generic_web"){
304-
hideShowByClassName(".source-generic-web", '');
315+
var contentProvider = form.querySelector('input[name="content-provider"]:checked').value;
316+
hideShowByClassName(".content-provider", 'none');
317+
318+
if(contentProvider == 'git'){
319+
hideShowByClassName(".content-provider-git", '');
320+
} else if(contentProvider == 'googledrive'){
321+
hideShowByClassName(".content-provider-googledrive", '');
322+
} else if(contentProvider == 'dropbox'){
323+
hideShowByClassName(".content-provider-dropbox", '');
324+
} else if(contentProvider =="generic_web"){
325+
hideShowByClassName(".content-provider-generic-web", '');
305326
}
306327
}
307328

@@ -345,10 +366,10 @@ function main() {
345366
element.addEventListener('change', render);
346367
}
347368
)
348-
document.querySelectorAll('#linkgenerator input[name="source"]').forEach(
369+
document.querySelectorAll('#linkgenerator input[name="content-provider"]').forEach(
349370
function (element) {
350371
element.addEventListener('change', function(){
351-
displaySource();
372+
displayContentProvider();
352373
render();
353374
}
354375
);
@@ -375,7 +396,7 @@ function main() {
375396

376397

377398
// Do an initial render, to make sure our disabled / enabled properties are correctly set
378-
displaySource();
399+
displayContentProvider();
379400
render();
380401
}
381402

0 commit comments

Comments
 (0)