You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by michellephung <gi...@git.apache.org> on 2015/02/04 00:50:09 UTC
[GitHub] couchdb-fauxton pull request: Centralize URLs
GitHub user michellephung opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/258
Centralize URLs
COUCHDB-2566 centralization of URLs used in Fauxton.
@robertkowalski I'm having a bit of trouble passing the grunt test. Could you take a quick look? The error I'm getting is
Error: A "url" property or function must be specified
which is weird, because I haven't taken the url property out of any of the collections or models.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/michellephung/couchdb-fauxton center-urls-copy
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/258.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #258
----
commit 384b2eda305f78805b8d2e2a93a9e069f800c137
Author: Michelle Phung <mi...@gmail.com>
Date: 2014-11-20T14:56:51Z
Centralize URLs
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74502288
merged as 92834eae02eadc361de3a21ee1079d8f47af6492 - thanks :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-73141347
- a more centralized system for retrieving, setting, and navigating URLs.
- Currently urls needed for Fauxton depend on the url specified within
each model. But a URL is essentially a string, so my proposal is that we
re-write the retrieval for each URL into one file.
- This should help when deciding what kind of URL a developer would need
for each specific task, and also help in debugging if they are working
with URLs.
Jira: 2566
and....
PASSING TESTS!! =)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-72836652
Hi Michelle,
can you describe whats the use case is and why this is a improvement?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74116949
@robertkowalski
I just added a nightwatch test, and re-added compact & clean
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74058385
@michellephung I just wanted to push to upstream, but I noticed that the "compact & clean" link broke.
Can you fix the issue and write a test using nightwatch so the regression never happens again?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74236149
hi @michellephung are you sure you have committed the new file? it seems it is missing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-73229701
super cool! just found minor nits, i think we are ready to go afterwards.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74189968
@benkeen @robertkowalski
hey could you guys take a look at this ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74299840
This could be unrelated, but when testing in the UI, I found a problem.
When in a database on the All Docs page, I clicked onto the Compact & Clean page then back to All Docs - the Compact & Clean page content stays there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24682607
--- Diff: app/addons/documents/shared-resources.js ---
@@ -242,14 +241,10 @@ define([
} else if (this.params) {
query = "?" + $.param(this.params);
}
-
- if (context === 'app') {
- return 'database/' + this.database.safeID() + "/_all_docs" + query;
- } else if (context === "apiurl"){
- return window.location.origin + "/" + this.database.safeID() + "/_all_docs" + query;
- } else {
- return app.host + "/" + this.database.safeID() + "/_all_docs" + query;
+ if (context === undefined) {
--- End diff --
_.undefined
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74500462
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24683640
--- Diff: app/addons/documents/resources.js ---
@@ -216,11 +214,18 @@ function(app, FauxtonAPI, Documents, PagingCollection) {
} else if (context === "apiurl"){
startOfUrl = window.location.origin;
}
- var design = app.utils.safeURLName(this.design),
- view = app.utils.safeURLName(this.view);
- var url = [startOfUrl, this.database.safeID(), "_design", design, this.idxType, view];
- return url.join("/") + query;
+ var database = this.database.safeID(),
+ design = app.utils.safeURLName(this.design),
+ view = app.utils.safeURLName(this.view),
+ url = FauxtonAPI.urls('view', 'server', database, design, view);
+
+ if (url === '') {
--- End diff --
I'd definitely look into this chunk. Does seem a bit fishy that url *may* be empty. Is that possible, with the new FauxtonAPI.urls method?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74275207
it's a short test: app/addons/compaction/tests/nightwatch/compactAndCleanIsPresent.js
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-73920466
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-73691181
@michellephung this looks good but the tests are still failing. We cannot merge this in until the tests are passing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24097373
--- Diff: app/addons/documents/templates/design_doc_menu.html ---
@@ -12,21 +12,21 @@
the License.
-->
<li class="nav-header">
-
- <div class="js-collapse-toggle accordion-header" data-toggle="collapse" data-target="#<%- ddoc_clean %>" id="nav-header-<%- ddoc_clean %>" >
- <div class="accordion-list-item">
- <div class="fonticon-play"></div>
- <p>_design/<%- designDoc%></p>
- </div>
- <div class="new-button add-dropdown"></div>
- </div>
- <ul class="accordion-body collapse" id="<%= ddoc_clean %>">
- <li>
- <a id="<%- ddoc_clean %>_metadata" href="#/database/<%- database_encoded %>/_design/<%- ddoc_encoded %>/metadata" class="toggle-view accordion-header">
- <span class="fonticon-sidenav-info fonticon"></span>
- Design Doc Metadata
- </a>
- </li>
- </ul>
+<div class="js-collapse-toggle accordion-header" data-toggle="collapse" data-target="#<%- ddoc_clean %>" id="nav-header-<%- ddoc_clean %>" >
+ <div class="accordion-list-item">
+ <div class="fonticon-play"></div>
+ <p>_design/<%- designDoc%></p>
+ </div>
+ <div class="new-button add-dropdown"></div>
+</div>
+<ul class="accordion-body collapse" id="<%= ddoc_clean %>">
--- End diff --
please use `<%-`for escaped content, XSS attack possible
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24097198
--- Diff: app/addons/documents/base.js ---
@@ -20,5 +20,90 @@ define([
],
function(app, FauxtonAPI, Documents) {
+
+ FauxtonAPI.registerUrls( 'allDocs', {
+ server: function (id, query) {
--- End diff --
:space_invader:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24682388
--- Diff: app/addons/documents/routes-documents.js ---
@@ -98,12 +98,17 @@ function(app, FauxtonAPI, BaseRoute, Documents, Changes, Index, DocEditor, Datab
this.addSidebar();
},
+ getAllDatabases: function () {
--- End diff --
this is done so it can be overwritten
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/258
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#issuecomment-74345225
I think i fixed all of changes. Waiting for Travis.
When I checked, Compact/Clean was working ok.
Please let me know if its acting up for you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Centralize URLs
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/258#discussion_r24683452
--- Diff: app/addons/documents/routes-documents.js ---
@@ -98,12 +98,17 @@ function(app, FauxtonAPI, BaseRoute, Documents, Changes, Index, DocEditor, Datab
this.addSidebar();
},
+ getAllDatabases: function () {
--- End diff --
Mind adding in a comment in the code itself saying why it's done - I'd look at it and think it was unnecessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---