Commit 6e896a5f authored by Mike Jang's avatar Mike Jang

Merge branch 'mjang-remove-future-tense' into 'master'

Remove future tense from Manage Stage docs in the doc/development

See merge request gitlab-org/gitlab!49039
parents 90f3b69a af70b17f
...@@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
For working with internationalization (i18n), For working with internationalization (i18n),
[GNU gettext](https://www.gnu.org/software/gettext/) is used given it's the most [GNU gettext](https://www.gnu.org/software/gettext/) is used given it's the most
used tool for this task and there are a lot of applications that will help us to used tool for this task and there are a lot of applications that help us
work with it. work with it.
TIP: **Tip:** TIP: **Tip:**
...@@ -376,7 +376,7 @@ Namespaces should be PascalCase. ...@@ -376,7 +376,7 @@ Namespaces should be PascalCase.
s_('OpenedNDaysAgo|Opened') s_('OpenedNDaysAgo|Opened')
``` ```
In case the translation is not found it will return `Opened`. In case the translation is not found it returns `Opened`.
- In JavaScript: - In JavaScript:
...@@ -417,12 +417,12 @@ To include formatting in the translated string, we can do the following: ...@@ -417,12 +417,12 @@ To include formatting in the translated string, we can do the following:
See the section on [interpolation](#interpolation). See the section on [interpolation](#interpolation).
When [this translation helper issue](https://gitlab.com/gitlab-org/gitlab/-/issues/217935) is complete, we'll update the When [this translation helper issue](https://gitlab.com/gitlab-org/gitlab/-/issues/217935) is complete, we plan to update the
process of including formatting in translated strings. process of including formatting in translated strings.
#### Including Angle Brackets #### Including Angle Brackets
If a string contains angles brackets (`<`/`>`) that are not used for HTML, it will still be flagged by the If a string contains angles brackets (`<`/`>`) that are not used for HTML, it is still flagged by the
`rake gettext:lint` linter. `rake gettext:lint` linter.
To avoid this error, use the applicable HTML entity code (`&lt;` or `&gt;`) instead: To avoid this error, use the applicable HTML entity code (`&lt;` or `&gt;`) instead:
...@@ -473,7 +473,7 @@ This makes use of [`Intl.DateTimeFormat`](https://developer.mozilla.org/en-US/do ...@@ -473,7 +473,7 @@ This makes use of [`Intl.DateTimeFormat`](https://developer.mozilla.org/en-US/do
1. **Through the `l` helper**, i.e. `l(active_session.created_at, format: :short)`. We have some predefined formats for 1. **Through the `l` helper**, i.e. `l(active_session.created_at, format: :short)`. We have some predefined formats for
[dates](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L262). [dates](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L262).
If you need to add a new format, because other parts of the code could benefit from it, If you need to add a new format, because other parts of the code could benefit from it,
you'll need to add it to [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) file. you can add it to [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) file.
1. **Through `strftime`**, i.e. `milestone.start_date.strftime('%b %-d')`. We use `strftime` in case none of the formats 1. **Through `strftime`**, i.e. `milestone.start_date.strftime('%b %-d')`. We use `strftime` in case none of the formats
defined on [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) matches the date/time defined on [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) matches the date/time
specifications we need, and if there is no need to add it as a new format because is very particular (i.e. it's only used in a single view). specifications we need, and if there is no need to add it as a new format because is very particular (i.e. it's only used in a single view).
...@@ -504,7 +504,7 @@ Examples: ...@@ -504,7 +504,7 @@ Examples:
- Mappings for a dropdown list - Mappings for a dropdown list
- Error messages - Error messages
To store these kinds of data, using a constant seems like the best choice, however this won't work for translations. To store these kinds of data, using a constant seems like the best choice, however this doesn't work for translations.
Bad, avoid it: Bad, avoid it:
...@@ -518,7 +518,7 @@ class MyPresenter ...@@ -518,7 +518,7 @@ class MyPresenter
end end
``` ```
The translation method (`_`) will be called when the class is loaded for the first time and translates the text to the default locale. Regardless of what's the user's locale, these values will not be translated again. The translation method (`_`) is called when the class is loaded for the first time and translates the text to the default locale. Regardless of the user's locale, these values are not translated a second time.
Similar thing happens when using class methods with memoization. Similar thing happens when using class methods with memoization.
...@@ -536,7 +536,7 @@ class MyModel ...@@ -536,7 +536,7 @@ class MyModel
end end
``` ```
This method will memoize the translations using the locale of the user, who first "called" this method. This method memorizes the translations using the locale of the user, who first "called" this method.
To avoid these problems, keep the translations dynamic. To avoid these problems, keep the translations dynamic.
...@@ -700,9 +700,9 @@ Now that the new content is marked for translation, we need to update ...@@ -700,9 +700,9 @@ Now that the new content is marked for translation, we need to update
bin/rake gettext:regenerate bin/rake gettext:regenerate
``` ```
This command will update `locale/gitlab.pot` file with the newly externalized This command updates `locale/gitlab.pot` file with the newly externalized
strings and remove any strings that aren't used anymore. You should check this strings and remove any strings that aren't used anymore. You should check this
file in. Once the changes are on master, they will be picked up by file in. Once the changes are on master, they are picked up by
[CrowdIn](https://translate.gitlab.com) and be presented for [CrowdIn](https://translate.gitlab.com) and be presented for
translation. translation.
...@@ -719,7 +719,7 @@ running on CI as part of the `static-analysis` job. ...@@ -719,7 +719,7 @@ running on CI as part of the `static-analysis` job.
To lint the adjustments in PO files locally you can run `rake gettext:lint`. To lint the adjustments in PO files locally you can run `rake gettext:lint`.
The linter will take the following into account: The linter takes the following into account:
- Valid PO-file syntax - Valid PO-file syntax
- Variable usage - Variable usage
...@@ -758,7 +758,7 @@ aren't in the message with ID `1 pipeline`. ...@@ -758,7 +758,7 @@ aren't in the message with ID `1 pipeline`.
NOTE: NOTE:
[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/221012) in GitLab 13.3: [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/221012) in GitLab 13.3:
Languages with less than 2% of translations won't be available in the UI. Languages with less than 2% of translations are not available in the UI.
Let's suppose you want to add translations for a new language, let's say French. Let's suppose you want to add translations for a new language, let's say French.
......
...@@ -9,13 +9,13 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -9,13 +9,13 @@ info: To determine the technical writer assigned to the Stage/Group associated w
The text in GitLab's user interface is in American English by default. The text in GitLab's user interface is in American English by default.
Each string can be translated to other languages. Each string can be translated to other languages.
As each string is translated, it is added to the languages translation file, As each string is translated, it is added to the languages translation file,
and will be available in future releases of GitLab. and is made available in future releases of GitLab.
Contributions to translations are always needed. Contributions to translations are always needed.
Many strings are not yet available for translation because they have not been externalized. Many strings are not yet available for translation because they have not been externalized.
Helping externalize strings benefits all languages. Helping externalize strings benefits all languages.
Some translations are incomplete or inconsistent. Some translations are incomplete or inconsistent.
Translating strings will help complete and improve each language. Translating strings helps complete and improve each language.
## How to contribute ## How to contribute
...@@ -37,7 +37,7 @@ See [Externalization for GitLab](externalization.md). ...@@ -37,7 +37,7 @@ See [Externalization for GitLab](externalization.md).
The translation process is managed at <https://translate.gitlab.com> The translation process is managed at <https://translate.gitlab.com>
using [CrowdIn](https://crowdin.com/). using [CrowdIn](https://crowdin.com/).
You will need to create an account before you can submit translations. You need to create an account before you can submit translations.
Once you are signed in, select the language you wish to contribute translations to. Once you are signed in, select the language you wish to contribute translations to.
Voting for translations is also valuable, helping to confirm good and flag inaccurate translations. Voting for translations is also valuable, helping to confirm good and flag inaccurate translations.
...@@ -48,7 +48,7 @@ See [Translation guidelines](translation.md). ...@@ -48,7 +48,7 @@ See [Translation guidelines](translation.md).
Proofreading helps ensure the accuracy and consistency of translations. All Proofreading helps ensure the accuracy and consistency of translations. All
translations are proofread before being accepted. If a translations requires translations are proofread before being accepted. If a translations requires
changes, you will be notified with a comment explaining why. changes, you are notified with a comment explaining why.
See [Proofreading Translations](proofreader.md) for more information on who's See [Proofreading Translations](proofreader.md) for more information on who's
able to proofread and instructions on becoming a proofreader yourself. able to proofread and instructions on becoming a proofreader yourself.
......
...@@ -27,8 +27,8 @@ If there are validation errors, the easiest solution is to disapprove ...@@ -27,8 +27,8 @@ If there are validation errors, the easiest solution is to disapprove
the offending string in CrowdIn, leaving a comment with what is the offending string in CrowdIn, leaving a comment with what is
required to fix the offense. There is an required to fix the offense. There is an
[issue](https://gitlab.com/gitlab-org/gitlab/-/issues/23256) [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/23256)
suggesting to automate this process. Disapproving will exclude the suggesting to automate this process. Disapproving excludes the
invalid translation, the merge request will be updated within a few invalid translation, the merge request is then updated within a few
minutes. minutes.
If the translation has failed validation due to angle brackets `<` or `>` If the translation has failed validation due to angle brackets `<` or `>`
...@@ -52,7 +52,7 @@ We are discussing [automating this entire process](https://gitlab.com/gitlab-org ...@@ -52,7 +52,7 @@ We are discussing [automating this entire process](https://gitlab.com/gitlab-org
## Recreate the merge request ## Recreate the merge request
CrowdIn creates a new merge request as soon as the old one is closed CrowdIn creates a new merge request as soon as the old one is closed
or merged. But it won't recreate the `master-i18n` branch every or merged. But it does not recreate the `master-i18n` branch every
time. To force CrowdIn to recreate the branch, close any [open merge time. To force CrowdIn to recreate the branch, close any [open merge
request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&author_username=gitlab-crowdin-bot) request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&author_username=gitlab-crowdin-bot)
and delete the and delete the
......
...@@ -147,9 +147,9 @@ of contributing translations to the GitLab project. ...@@ -147,9 +147,9 @@ of contributing translations to the GitLab project.
In the merge request description, include links to any projects you have In the merge request description, include links to any projects you have
previously translated. previously translated.
1. Your request to become a proofreader will be considered on the merits of 1. Your request to become a proofreader is considered on the merits of
your previous translations by [GitLab team members](https://about.gitlab.com/company/team/) your previous translations by [GitLab team members](https://about.gitlab.com/company/team/)
or [Core team members](https://about.gitlab.com/community/core-team/) who are fluent in or [Core team members](https://about.gitlab.com/community/core-team/) who are fluent in
the language or current proofreaders. the language or current proofreaders.
- When a request is made for the first proofreader for a language and there are no [GitLab team members](https://about.gitlab.com/company/team/) - When a request is made for the first proofreader for a language and there are no [GitLab team members](https://about.gitlab.com/company/team/)
or [Core team members](https://about.gitlab.com/community/core-team/) who speak the language, we will request links to previous translation work in other communities or projects. or [Core team members](https://about.gitlab.com/community/core-team/) who speak the language, we shall request links to previous translation work in other communities or projects.
...@@ -23,7 +23,7 @@ You may create a new account or use any of their supported sign in services. ...@@ -23,7 +23,7 @@ You may create a new account or use any of their supported sign in services.
GitLab is being translated into many languages. GitLab is being translated into many languages.
1. Select the language you would like to contribute translations to by clicking the flag 1. Select the language you would like to contribute translations to by clicking the flag
1. You will see a list of files and folders. 1. Next, you can view list of files and folders.
Click `gitlab.pot` to open the translation editor. Click `gitlab.pot` to open the translation editor.
### Translation Editor ### Translation Editor
...@@ -34,7 +34,7 @@ The online translation editor is the easiest way to contribute translations. ...@@ -34,7 +34,7 @@ The online translation editor is the easiest way to contribute translations.
1. Strings for translation are listed in the left panel 1. Strings for translation are listed in the left panel
1. Translations are entered into the central panel. 1. Translations are entered into the central panel.
Multiple translations will be required for strings that contains plurals. Multiple translations are required for strings that contains plurals.
The string to be translated is shown above with glossary terms highlighted. The string to be translated is shown above with glossary terms highlighted.
If the string to be translated is not clear, you can 'Request Context' If the string to be translated is not clear, you can 'Request Context'
......
...@@ -42,7 +42,7 @@ SIDEKIQ_MEMORY_KILLER_HARD_LIMIT_RSS = 3000000 ...@@ -42,7 +42,7 @@ SIDEKIQ_MEMORY_KILLER_HARD_LIMIT_RSS = 3000000
SIDEKIQ_MEMORY_KILLER_GRACE_TIME = 900 SIDEKIQ_MEMORY_KILLER_GRACE_TIME = 900
``` ```
An import status `started`, and the following Sidekiq logs will signal a memory issue: An import status `started`, and the following Sidekiq logs signal a memory issue:
```shell ```shell
WARN: Work still in progress <struct with JID> WARN: Work still in progress <struct with JID>
...@@ -218,7 +218,7 @@ for compatibility when importing and exporting projects. ...@@ -218,7 +218,7 @@ for compatibility when importing and exporting projects.
### When to bump the version up ### When to bump the version up
We will have to bump the version if we rename model/columns or perform any format If we rename model/columns or perform any format, we need to bump the version
modifications in the JSON structure or the file structure of the archive file. modifications in the JSON structure or the file structure of the archive file.
We do not need to bump the version up in any of the following cases: We do not need to bump the version up in any of the following cases:
...@@ -227,7 +227,7 @@ We do not need to bump the version up in any of the following cases: ...@@ -227,7 +227,7 @@ We do not need to bump the version up in any of the following cases:
- Remove a column or model (unless there is a DB constraint) - Remove a column or model (unless there is a DB constraint)
- Export new things (such as a new type of upload) - Export new things (such as a new type of upload)
Every time we bump the version, the integration specs will fail and can be fixed with: Every time we bump the version, the integration specs fail and can be fixed with:
```shell ```shell
bundle exec rake gitlab:import_export:bump_version bundle exec rake gitlab:import_export:bump_version
...@@ -355,7 +355,7 @@ The tools to generate the NDJSON tree from the human-readable JSON files live in ...@@ -355,7 +355,7 @@ The tools to generate the NDJSON tree from the human-readable JSON files live in
**Please use `legacy-project-json-to-ndjson.sh` to generate the NDJSON tree.** **Please use `legacy-project-json-to-ndjson.sh` to generate the NDJSON tree.**
The NDJSON tree will look like this: The NDJSON tree looks like:
```shell ```shell
tree tree
...@@ -389,7 +389,7 @@ tree ...@@ -389,7 +389,7 @@ tree
**Please use `legacy-group-json-to-ndjson.rb` to generate the NDJSON tree.** **Please use `legacy-group-json-to-ndjson.rb` to generate the NDJSON tree.**
The NDJSON tree will look like this: The NDJSON tree looks like this:
```shell ```shell
tree tree
......
...@@ -72,7 +72,7 @@ is stored in the `project_authorizations` table. ...@@ -72,7 +72,7 @@ is stored in the `project_authorizations` table.
WARNING: WARNING:
Due to [an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299), Due to [an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299),
projects in personal namespace will not show owner (`50`) permission in projects in personal namespace do not show owner (`50`) permission in
`project_authorizations` table. Note however that [`user.owned_projects`](https://gitlab.com/gitlab-org/gitlab/blob/0d63823b122b11abd2492bca47cc26858eee713d/app/models/user.rb#L906-916) `project_authorizations` table. Note however that [`user.owned_projects`](https://gitlab.com/gitlab-org/gitlab/blob/0d63823b122b11abd2492bca47cc26858eee713d/app/models/user.rb#L906-916)
is calculated properly. is calculated properly.
...@@ -98,7 +98,7 @@ In the case of a complex resource, it should be broken into smaller pieces of in ...@@ -98,7 +98,7 @@ In the case of a complex resource, it should be broken into smaller pieces of in
and each piece should be granted a different permission. and each piece should be granted a different permission.
A good example in this case is the _Merge Request widget_ and the _Security reports_. A good example in this case is the _Merge Request widget_ and the _Security reports_.
Depending on the visibility level of the _Pipelines_, the _Security reports_ will be either visible Depending on the visibility level of the _Pipelines_, the _Security reports_ are either visible
in the widget or not. So, the _Merge Request widget_, the _Pipelines_, and the _Security reports_, in the widget or not. So, the _Merge Request widget_, the _Pipelines_, and the _Security reports_,
have separate permissions. Moreover, the permissions for the _Merge Request widget_ have separate permissions. Moreover, the permissions for the _Merge Request widget_
and the _Pipelines_ are dependencies of the _Security reports_. and the _Pipelines_ are dependencies of the _Security reports_.
......
...@@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
The DeclarativePolicy framework is designed to assist in performance of policy checks, and to enable ease of extension for EE. The DSL code in `app/policies` is what `Ability.allowed?` uses to check whether a particular action is allowed on a subject. The DeclarativePolicy framework is designed to assist in performance of policy checks, and to enable ease of extension for EE. The DSL code in `app/policies` is what `Ability.allowed?` uses to check whether a particular action is allowed on a subject.
The policy used is based on the subject's class name - so `Ability.allowed?(user, :some_ability, project)` will create a `ProjectPolicy` and check permissions on that. The policy used is based on the subject's class name - so `Ability.allowed?(user, :some_ability, project)` creates a `ProjectPolicy` and check permissions on that.
## Managing Permission Rules ## Managing Permission Rules
...@@ -16,7 +16,7 @@ Permissions are broken into two parts: `conditions` and `rules`. Conditions are ...@@ -16,7 +16,7 @@ Permissions are broken into two parts: `conditions` and `rules`. Conditions are
### Conditions ### Conditions
Conditions are defined by the `condition` method, and are given a name and a block. The block will be executed in the context of the policy object - so it can access `@user` and `@subject`, as well as call any methods defined on the policy. Note that `@user` may be nil (in the anonymous case), but `@subject` is guaranteed to be a real instance of the subject class. Conditions are defined by the `condition` method, and are given a name and a block. The block is executed in the context of the policy object - so it can access `@user` and `@subject`, as well as call any methods defined on the policy. Note that `@user` may be nil (in the anonymous case), but `@subject` is guaranteed to be a real instance of the subject class.
```ruby ```ruby
class FooPolicy < BasePolicy class FooPolicy < BasePolicy
...@@ -34,9 +34,9 @@ class FooPolicy < BasePolicy ...@@ -34,9 +34,9 @@ class FooPolicy < BasePolicy
end end
``` ```
When you define a condition, a predicate method is defined on the policy to check whether that condition passes - so in the above example, an instance of `FooPolicy` will also respond to `#is_public?` and `#thing?`. When you define a condition, a predicate method is defined on the policy to check whether that condition passes - so in the above example, an instance of `FooPolicy` also responds to `#is_public?` and `#thing?`.
Conditions are cached according to their scope. Scope and ordering will be covered later. Conditions are cached according to their scope. Scope and ordering is covered later.
### Rules ### Rules
...@@ -69,7 +69,7 @@ Within the rule DSL, you can use: ...@@ -69,7 +69,7 @@ Within the rule DSL, you can use:
## Scores, Order, Performance ## Scores, Order, Performance
To see how the rules get evaluated into a judgment, it is useful in a console to use `policy.debug(:some_ability)`. This will print the rules in the order they are evaluated. To see how the rules get evaluated into a judgment, it is useful in a console to use `policy.debug(:some_ability)`. This prints the rules in the order they are evaluated.
For example, let's say you wanted to debug `IssuePolicy`. You might run For example, let's say you wanted to debug `IssuePolicy`. You might run
the debugger in this way: the debugger in this way:
...@@ -109,9 +109,9 @@ When a policy is asked whether a particular ability is allowed ...@@ -109,9 +109,9 @@ When a policy is asked whether a particular ability is allowed
compute all the conditions on the policy. First, only the rules relevant compute all the conditions on the policy. First, only the rules relevant
to that particular ability are selected. Then, the execution model takes to that particular ability are selected. Then, the execution model takes
advantage of short-circuiting, and attempts to sort rules based on a advantage of short-circuiting, and attempts to sort rules based on a
heuristic of how expensive they will be to calculate. The sorting is heuristic of how expensive they are to calculate. The sorting is
dynamic and cache-aware, so that previously calculated conditions will dynamic and cache-aware, so that previously calculated conditions are
be considered first, before computing other conditions. considered first, before computing other conditions.
Note that the score is chosen by a developer via the `score:` parameter Note that the score is chosen by a developer via the `score:` parameter
in a `condition` to denote how expensive evaluating this rule would be in a `condition` to denote how expensive evaluating this rule would be
...@@ -119,7 +119,7 @@ relative to other rules. ...@@ -119,7 +119,7 @@ relative to other rules.
## Scope ## Scope
Sometimes, a condition will only use data from `@user` or only from `@subject`. In this case, we want to change the scope of the caching, so that we don't recalculate conditions unnecessarily. For example, given: Sometimes, a condition only uses data from `@user` or only from `@subject`. In this case, we want to change the scope of the caching, so that we don't recalculate conditions unnecessarily. For example, given:
```ruby ```ruby
class FooPolicy < BasePolicy class FooPolicy < BasePolicy
...@@ -135,10 +135,10 @@ Naively, if we call `Ability.allowed?(user1, :some_ability, foo)` and `Ability.a ...@@ -135,10 +135,10 @@ Naively, if we call `Ability.allowed?(user1, :some_ability, foo)` and `Ability.a
condition(:expensive_condition, scope: :subject) { @subject.expensive_query? } condition(:expensive_condition, scope: :subject) { @subject.expensive_query? }
``` ```
then the result of the condition will be cached globally only based on the subject - so it will not be calculated repeatedly for different users. Similarly, `scope: :user` will cache only based on the user. then the result of the condition is cached globally only based on the subject - so it is not calculated repeatedly for different users. Similarly, `scope: :user` caches only based on the user.
**DANGER**: If you use a `:scope` option when the condition actually uses data from **DANGER**: If you use a `:scope` option when the condition actually uses data from
both user and subject (including a simple anonymous check!) your result will be cached at too global of a scope and will result in cache bugs. both user and subject (including a simple anonymous check!) your result is cached at too global of a scope and results in cache bugs.
Sometimes we are checking permissions for a lot of users for one subject, or a lot of subjects for one user. In this case, we want to set a *preferred scope* - i.e. tell the system that we prefer rules that can be cached on the repeated parameter. For example, in `Ability.users_that_can_read_project`: Sometimes we are checking permissions for a lot of users for one subject, or a lot of subjects for one user. In this case, we want to set a *preferred scope* - i.e. tell the system that we prefer rules that can be cached on the repeated parameter. For example, in `Ability.users_that_can_read_project`:
...@@ -150,7 +150,7 @@ def users_that_can_read_project(users, project) ...@@ -150,7 +150,7 @@ def users_that_can_read_project(users, project)
end end
``` ```
This will, for example, prefer checking `project.public?` to checking `user.admin?`. This, for example, prefers checking `project.public?` to checking `user.admin?`.
## Delegation ## Delegation
...@@ -162,7 +162,7 @@ class FooPolicy < BasePolicy ...@@ -162,7 +162,7 @@ class FooPolicy < BasePolicy
end end
``` ```
will include all rules from `ProjectPolicy`. The delegated conditions will be evaluated with the correct delegated subject, and will be sorted along with the regular rules in the policy. Note that only the relevant rules for a particular ability will actually be considered. includes all rules from `ProjectPolicy`. The delegated conditions are evaluated with the correct delegated subject, and are sorted along with the regular rules in the policy. Note that only the relevant rules for a particular ability are actually considered.
### Overrides ### Overrides
...@@ -203,7 +203,7 @@ end ...@@ -203,7 +203,7 @@ end
But the food preferences one is harder - because of the `prevent` call in the But the food preferences one is harder - because of the `prevent` call in the
parent policy, if the parent dislikes it, even calling `enable` in the child parent policy, if the parent dislikes it, even calling `enable` in the child
will not enable `:eat_broccoli`. does not enable `:eat_broccoli`.
We could remove the `prevent` call in the parent policy, but that still doesn't We could remove the `prevent` call in the parent policy, but that still doesn't
help us, since the rules are different: parents get to eat what they like, and help us, since the rules are different: parents get to eat what they like, and
...@@ -226,7 +226,7 @@ class ChildPolicy < BasePolicy ...@@ -226,7 +226,7 @@ class ChildPolicy < BasePolicy
end end
``` ```
With this definition, the `ChildPolicy` will _never_ look in the `ParentPolicy` to With this definition, the `ChildPolicy` _never_ looks in the `ParentPolicy` to
satisfy `:eat_broccoli`, but it _will_ use it for any other abilities. The child satisfy `:eat_broccoli`, but it _will_ use it for any other abilities. The child
policy can then define `:eat_broccoli` in a way that makes sense for `Child` and not policy can then define `:eat_broccoli` in a way that makes sense for `Child` and not
`Parent`. `Parent`.
...@@ -243,7 +243,7 @@ Other approaches can include for example using different ability names. Choosing ...@@ -243,7 +243,7 @@ Other approaches can include for example using different ability names. Choosing
to eat a food and eating foods you are given are semantically distinct, and they to eat a food and eating foods you are given are semantically distinct, and they
could be named differently (perhaps `chooses_to_eat_broccoli` and could be named differently (perhaps `chooses_to_eat_broccoli` and
`eats_what_is_given` in this case). It can depend on how polymorphic the call `eats_what_is_given` in this case). It can depend on how polymorphic the call
site is. If you know that we will always check the policy with a `Parent` or a site is. If you know that we always check the policy with a `Parent` or a
`Child`, then we can choose the appropriate ability name. If the call site is `Child`, then we can choose the appropriate ability name. If the call site is
polymorphic, then we cannot do that. polymorphic, then we cannot do that.
...@@ -260,4 +260,4 @@ class Foo ...@@ -260,4 +260,4 @@ class Foo
end end
``` ```
This will use & check permissions on the `SomeOtherPolicy` class rather than the usual calculated `FooPolicy` class. This uses and checks permissions on the `SomeOtherPolicy` class rather than the usual calculated `FooPolicy` class.
...@@ -32,7 +32,7 @@ These events play a key role in the duration calculation. ...@@ -32,7 +32,7 @@ These events play a key role in the duration calculation.
Formula: `duration = end_event_time - start_event_time` Formula: `duration = end_event_time - start_event_time`
To make the duration calculation flexible, each `Event` is implemented as a separate class. They're responsible for defining a timestamp expression that will be used in the calculation query. To make the duration calculation flexible, each `Event` is implemented as a separate class. They're responsible for defining a timestamp expression that is used in the calculation query.
#### Implementing an `Event` class #### Implementing an `Event` class
...@@ -41,12 +41,12 @@ There are a few methods that are required to be implemented, the `StageEvent` ba ...@@ -41,12 +41,12 @@ There are a few methods that are required to be implemented, the `StageEvent` ba
- `object_type` - `object_type`
- `timestamp_projection` - `timestamp_projection`
The `object_type` method defines which domain object will be queried for the calculation. Currently two models are allowed: The `object_type` method defines which domain object is queried for the calculation. Currently two models are allowed:
- `Issue` - `Issue`
- `MergeRequest` - `MergeRequest`
For the duration calculation the `timestamp_projection` method will be used. For the duration calculation the `timestamp_projection` method is used.
```ruby ```ruby
def timestamp_projection def timestamp_projection
...@@ -92,7 +92,7 @@ Some start/end event pairs are not "compatible" with each other. For example: ...@@ -92,7 +92,7 @@ Some start/end event pairs are not "compatible" with each other. For example:
The `StageEvents` module describes the allowed `start_event` and `end_event` pairings (`PAIRING_RULES` constant). If a new event is added, it needs to be registered in this module. The `StageEvents` module describes the allowed `start_event` and `end_event` pairings (`PAIRING_RULES` constant). If a new event is added, it needs to be registered in this module.
​To add a new event:​ ​To add a new event:​
1. Add an entry in `ENUM_MAPPING` with a unique number, it'll be used in the `Stage` model as `enum`. 1. Add an entry in `ENUM_MAPPING` with a unique number, which is used in the `Stage` model as `enum`.
1. Define which events are compatible with the event in the `PAIRING_RULES` hash. 1. Define which events are compatible with the event in the `PAIRING_RULES` hash.
Supported start/end event pairings: Supported start/end event pairings:
...@@ -185,19 +185,19 @@ Currently supported parents: ...@@ -185,19 +185,19 @@ Currently supported parents:
1. User navigates to the value stream analytics page. 1. User navigates to the value stream analytics page.
1. User selects a group. 1. User selects a group.
1. Backend loads the defined stages for the selected group. 1. Backend loads the defined stages for the selected group.
1. Additions and modifications to the stages will be persisted within the selected group only. 1. Additions and modifications to the stages are persisted within the selected group only.
### Default stages ### Default stages
The [original implementation](https://gitlab.com/gitlab-org/gitlab/-/issues/847) of value stream analytics defined 7 stages. These stages are always available for each parent, however altering these stages is not possible. The [original implementation](https://gitlab.com/gitlab-org/gitlab/-/issues/847) of value stream analytics defined 7 stages. These stages are always available for each parent, however altering these stages is not possible.
To make things efficient and reduce the number of records created, the default stages are expressed as in-memory objects (not persisted). When the user creates a custom stage for the first time, all the stages will be persisted. This behavior is implemented in the value stream analytics service objects. To make things efficient and reduce the number of records created, the default stages are expressed as in-memory objects (not persisted). When the user creates a custom stage for the first time, all the stages are persisted. This behavior is implemented in the value stream analytics service objects.
The reason for this was that we'd like to add the abilities to hide and order stages later on. The reason for this was that we'd like to add the abilities to hide and order stages later on.
## Data Collector ## Data Collector
`DataCollector` is the central point where the data will be queried from the database. The class always operates on a single stage and consists of the following components: `DataCollector` is the central point where the data is queried from the database. The class always operates on a single stage and consists of the following components:
- `BaseQueryBuilder`: - `BaseQueryBuilder`:
- Responsible for composing the initial query. - Responsible for composing the initial query.
...@@ -238,7 +238,7 @@ SELECT (START_EVENT_TIME-END_EVENT_TIME) as duration, END_EVENT.timestamp ...@@ -238,7 +238,7 @@ SELECT (START_EVENT_TIME-END_EVENT_TIME) as duration, END_EVENT.timestamp
## High-level overview ## High-level overview
- Rails Controller (`Analytics::CycleAnalytics` module): Value stream analytics exposes its data via JSON endpoints, implemented within the `analytics` workspace. Configuring the stages are also implements JSON endpoints (CRUD). - Rails Controller (`Analytics::CycleAnalytics` module): Value stream analytics exposes its data via JSON endpoints, implemented within the `analytics` workspace. Configuring the stages are also implements JSON endpoints (CRUD).
- Services (`Analytics::CycleAnalytics` module): All `Stage` related actions will be delegated to respective service objects. - Services (`Analytics::CycleAnalytics` module): All `Stage` related actions are delegated to respective service objects.
- Models (`Analytics::CycleAnalytics` module): Models are used to persist the `Stage` objects `ProjectStage` and `GroupStage`. - Models (`Analytics::CycleAnalytics` module): Models are used to persist the `Stage` objects `ProjectStage` and `GroupStage`.
- Feature classes (`Gitlab::Analytics::CycleAnalytics` module): - Feature classes (`Gitlab::Analytics::CycleAnalytics` module):
- Responsible for composing queries and define feature specific business logic. - Responsible for composing queries and define feature specific business logic.
...@@ -248,7 +248,7 @@ SELECT (START_EVENT_TIME-END_EVENT_TIME) as duration, END_EVENT.timestamp ...@@ -248,7 +248,7 @@ SELECT (START_EVENT_TIME-END_EVENT_TIME) as duration, END_EVENT.timestamp
Since we have a lots of events and possible pairings, testing each pairing is not possible. The rule is to have at least one test case using an `Event` class. Since we have a lots of events and possible pairings, testing each pairing is not possible. The rule is to have at least one test case using an `Event` class.
Writing a test case for a stage using a new `Event` can be challenging since data must be created for both events. To make this a bit simpler, each test case must be implemented in the `data_collector_spec.rb` where the stage is tested through the `DataCollector`. Each test case will be turned into multiple tests, covering the following cases: Writing a test case for a stage using a new `Event` can be challenging since data must be created for both events. To make this a bit simpler, each test case must be implemented in the `data_collector_spec.rb` where the stage is tested through the `DataCollector`. Each test case is turned into multiple tests, covering the following cases:
- Different parents: `Group` or `Project` - Different parents: `Group` or `Project`
- Different calculations: `Median`, `RecordsFetcher` or `DataForDurationChart` - Different calculations: `Median`, `RecordsFetcher` or `DataForDurationChart`
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment