@@ -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 (`<` or `>`) instead:
To avoid this error, use the applicable HTML entity code (`<` or `>`) 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.
@@ -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.
@@ -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
classFooPolicy<BasePolicy
classFooPolicy<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
classFooPolicy<BasePolicy
classFooPolicy<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
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`:
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.
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
deftimestamp_projection
deftimestamp_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`.
- 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`