diff options
Diffstat (limited to 'doc/development')
| -rw-r--r-- | doc/development/fe_guide/index.md | 3 | ||||
| -rw-r--r-- | doc/development/fe_guide/vue.md | 51 | ||||
| -rw-r--r-- | doc/development/fe_guide/vue_resource.md | 72 | ||||
| -rw-r--r-- | doc/development/gitaly.md | 24 | ||||
| -rw-r--r-- | doc/development/licensing.md | 2 | ||||
| -rw-r--r-- | doc/development/profiling.md | 10 | ||||
| -rw-r--r-- | doc/development/testing_guide/best_practices.md | 2 |
7 files changed, 111 insertions, 53 deletions
diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index c0e1bfc12a1..73366eb9f3f 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -71,6 +71,9 @@ Vue specific design patterns and practices. --- +## [Vue Resource](vue_resource.md) +Vue resource specific practices and gotchas. + ## [Icons](icons.md) How we use SVG for our Icons. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 277e0cd5f00..f88f0753687 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -179,6 +179,7 @@ itself, please read this guide: [State Management][state-management] The Service is a class used only to communicate with the server. It does not store or manipulate any data. It is not aware of the store or the components. We use [vue-resource][vue-resource-repo] to communicate with the server. +Refer to [vue resource](vue_resource.md) for more details. Vue Resource should only be imported in the service file. @@ -189,55 +190,6 @@ Vue Resource should only be imported in the service file. Vue.use(VueResource); ``` -#### Vue-resource gotchas -#### Headers -Headers are being parsed into a plain object in an interceptor. -In Vue-resource 1.x `headers` object was changed into an `Headers` object. In order to not change all old code, an interceptor was added. - -If you need to write a unit test that takes the headers in consideration, you need to include an interceptor to parse the headers after your test interceptor. -You can see an example in `spec/javascripts/environments/environment_spec.js`: - ```javascript - import { headersInterceptor } from './helpers/vue_resource_helper'; - - beforeEach(() => { - Vue.http.interceptors.push(myInterceptor); - Vue.http.interceptors.push(headersInterceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, myInterceptor); - Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor); - }); - ``` - -#### `.json()` -When making a request to the server, you will most likely need to access the body of the response. -Use `.json()` to convert. Because `.json()` returns a Promise the follwoing structure should be used: - - ```javascript - service.get('url') - .then(resp => resp.json()) - .then((data) => { - this.store.storeData(data); - }) - .catch(() => new Flash('Something went wrong')); - ``` - -When using `Poll` (`app/assets/javascripts/lib/utils/poll.js`), the `successCallback` needs to handle `.json()` as a Promise: - ```javascript - successCallback: (response) => { - return response.json().then((data) => { - // handle the response - }); - } - ``` - -#### CSRF token -We use a Vue Resource interceptor to manage the CSRF token. -`app/assets/javascripts/vue_shared/vue_resource_interceptor.js` holds all our common interceptors. -Note: You don't need to load `app/assets/javascripts/vue_shared/vue_resource_interceptor.js` -since it's already being loaded by `common_vue.js`. - ### End Result The following example shows an application: @@ -769,7 +721,6 @@ describe('component', () => { [component-system]: https://vuejs.org/v2/guide/#Composing-with-Components [state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch [one-way-data-flow]: https://vuejs.org/v2/guide/components.html#One-Way-Data-Flow -[vue-resource-repo]: https://github.com/pagekit/vue-resource [vue-resource-interceptor]: https://github.com/pagekit/vue-resource/blob/develop/docs/http.md#interceptors [vue-test]: https://vuejs.org/v2/guide/unit-testing.html [issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 diff --git a/doc/development/fe_guide/vue_resource.md b/doc/development/fe_guide/vue_resource.md new file mode 100644 index 00000000000..c376c5c32bf --- /dev/null +++ b/doc/development/fe_guide/vue_resource.md @@ -0,0 +1,72 @@ +# Vue Resouce +In Vue applications we use [vue-resource][vue-resource-repo] to communicate with the server. + +## HTTP Status Codes + +### `.json()` +When making a request to the server, you will most likely need to access the body of the response. +Use `.json()` to convert. Because `.json()` returns a Promise the follwoing structure should be used: + + ```javascript + service.get('url') + .then(resp => resp.json()) + .then((data) => { + this.store.storeData(data); + }) + .catch(() => new Flash('Something went wrong')); + ``` + + +When using `Poll` (`app/assets/javascripts/lib/utils/poll.js`), the `successCallback` needs to handle `.json()` as a Promise: + ```javascript + successCallback: (response) => { + return response.json().then((data) => { + // handle the response + }); + } + ``` + +### 204 +Some endpoints - usually `delete` endpoints - return `204` as the success response. +When handling `204 - No Content` responses, we cannot use `.json()` since it tries to parse the non-existant body content. + +When handling `204` responses, do not use `.json`, otherwise the promise will throw an error and will enter the `catch` statement: + +```javascript + Vue.http.delete('path') + .then(() => { + // success! + }) + .catch(() => { + // handle error + }) +``` + +## Headers +Headers are being parsed into a plain object in an interceptor. +In Vue-resource 1.x `headers` object was changed into an `Headers` object. In order to not change all old code, an interceptor was added. + +If you need to write a unit test that takes the headers in consideration, you need to include an interceptor to parse the headers after your test interceptor. +You can see an example in `spec/javascripts/environments/environment_spec.js`: + ```javascript + import { headersInterceptor } from './helpers/vue_resource_helper'; + + beforeEach(() => { + Vue.http.interceptors.push(myInterceptor); + Vue.http.interceptors.push(headersInterceptor); + }); + + afterEach(() => { + Vue.http.interceptors = _.without(Vue.http.interceptors, myInterceptor); + Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor); + }); + ``` + +## CSRF token +We use a Vue Resource interceptor to manage the CSRF token. +`app/assets/javascripts/vue_shared/vue_resource_interceptor.js` holds all our common interceptors. +Note: You don't need to load `app/assets/javascripts/vue_shared/vue_resource_interceptor.js` +since it's already being loaded by `common_vue.js`. + + +[vue-resource-repo]: https://github.com/pagekit/vue-resource diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index e41d258bec6..ca2048c7019 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -52,8 +52,8 @@ rm -rf tmp/tests/gitaly ## `TooManyInvocationsError` errors -During development and testing, you may experience `Gitlab::GitalyClient::TooManyInvocationsError` failures. -The `GitalyClient` will attempt to block against potential n+1 issues by raising this error +During development and testing, you may experience `Gitlab::GitalyClient::TooManyInvocationsError` failures. +The `GitalyClient` will attempt to block against potential n+1 issues by raising this error when Gitaly is called more than 30 times in a single Rails request or Sidekiq execution. As a temporary measure, export `GITALY_DISABLE_REQUEST_LIMITS=1` to suppress the error. This will disable the n+1 detection @@ -64,7 +64,7 @@ Please raise an issue in the GitLab CE or EE repositories to report the issue. I `TooManyInvocationsError`. Also include any known failing tests if possible. Isolate the source of the n+1 problem. This will normally be a loop that results in Gitaly being called for each -element in an array. If you are unable to isolate the problem, please contact a member +element in an array. If you are unable to isolate the problem, please contact a member of the [Gitaly Team](https://gitlab.com/groups/gl-gitaly/group_members) for assistance. Once the source has been found, wrap it in an `allow_n_plus_1_calls` block, as follows: @@ -79,6 +79,24 @@ end Once the code is wrapped in this block, this code-path will be excluded from n+1 detection. +## Request counts + +Commits and other git data, is now fetched through Gitaly. These fetches can, +much like with a database, be batched. This improves performance for the client +and for Gitaly itself and therefore for the users too. To keep performance stable +and guard performance regressions, Gitaly calls can be counted and the call count +can be tested against. This requires the `:request_store` flag to be set. + +```ruby +describe 'Gitaly Request count tests' do + context 'when the request store is activated', :request_store do + it 'correctly counts the gitaly requests made' do + expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(10) + end + end +end +``` + --- [Return to Development documentation](README.md) diff --git a/doc/development/licensing.md b/doc/development/licensing.md index a75cdf22f40..902b1c74a42 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -56,6 +56,7 @@ Libraries with the following licenses are acceptable for use: - [ISC License][ISC] (also known as the OpenBSD License): A permissive (non-copyleft) license as defined by the Open Source Initiative. - [Creative Commons Zero (CC0)][CC0]: A public domain dedication, recommended as a way to disclaim copyright on your work to the maximum extent possible. - [Unlicense][UNLICENSE]: Another public domain dedication. +- [OWFa 1.0][OWFa1]: An open-source license and patent grant designed for specifications. ## Unacceptable Licenses @@ -105,6 +106,7 @@ Gems which are included only in the "development" or "test" groups by Bundler ar [OSL-GNU]: https://www.gnu.org/licenses/license-list.en.html#OSL [Org-Repo]: https://gitlab.com/gitlab-com/organization [UNLICENSE]: https://unlicense.org +[OWFa1]: http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0 [Facebook]: https://code.facebook.com/pages/850928938376556 [x-list]: https://www.apache.org/legal/resolved.html#category-x [Acceptable-Licenses]: #acceptable-licenses diff --git a/doc/development/profiling.md b/doc/development/profiling.md index 933033a09e0..af79353b721 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -27,3 +27,13 @@ Bullet will log query problems to both the Rails log as well as the Chrome console. As a follow up to finding `N+1` queries with Bullet, consider writing a [QueryRecoder test](query_recorder.md) to prevent a regression. + +## GitLab Profiler + + +[Gitlab-Profiler](https://gitlab.com/gitlab-com/gitlab-profiler) was built to +help developers understand why specific URLs of their application may be slow +and to provide hard data that can help reduce load times. + +For GitLab.com, you can find the latest results here: +<http://redash.gitlab.com/dashboard/gitlab-profiler-statistics> diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 613423dbd9a..7ddd02e6c73 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -35,6 +35,8 @@ Here are some things to keep in mind regarding test performance: [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). - Don't supply the `:each` argument to hooks since it's the default. - On `before` and `after` hooks, prefer it scoped to `:context` over `:all` +- When using `evaluate_script("$('.js-foo').testSomething()")` (or `execute_script`) which acts on a given element, + use a Capyabara matcher beforehand (e.g. `find('.js-foo')`) to ensure the element actually exists. [four-phase-test]: https://robots.thoughtbot.com/four-phase-test |