From 6f7d4096f53a0eecef559a075242a0af1443a332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Dec 2024 20:00:29 +0100 Subject: [PATCH] Add document about our coding conventions There are some conventions that can't be checked by linters and were not documented anywhere. --- CONTRIBUTING.md | 1 + CONTRIBUTING_ES.md | 1 + docs/en/SUMMARY.md | 1 + docs/en/customization/introduction.md | 4 + docs/en/open_source/coding_conventions.md | 150 ++++++++++++++++++++++ docs/en/open_source/contributing.md | 1 + docs/en/open_source/open_source.md | 1 + docs/es/SUMMARY.md | 1 + docs/es/customization/introduction.md | 4 + docs/es/open_source/coding_conventions.md | 150 ++++++++++++++++++++++ docs/es/open_source/contributing.md | 1 + docs/es/open_source/open_source.md | 1 + 12 files changed, 316 insertions(+) create mode 100644 docs/en/open_source/coding_conventions.md create mode 100644 docs/es/open_source/coding_conventions.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0af8d481..eca569bd1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,7 @@ If you'd like us to review your pull request in good spirits, please follow our * Run `bundle exec pronto run` and fix any issues it reports; these issues will also be automatically reported on the pull request * Follow [the seven rules of a great commit message](https://chris.beams.io/posts/git-commit/) * Add or modify i18n translations only in the base languages, English (en) and Spanish (es); we manage all other languages through the [Crowdin integration](https://translate.consuldemocracy.org/). +* Check the [coding conventions documentation file](docs/en/open_source/coding_conventions.md) for more information. When we review your pull request and ask for changes, if you're proficient using `git rebase` edit existing commits instead of adding new ones. If you aren't proficient with `git rebase`, ignore this point. diff --git a/CONTRIBUTING_ES.md b/CONTRIBUTING_ES.md index 39260358f..db104932c 100644 --- a/CONTRIBUTING_ES.md +++ b/CONTRIBUTING_ES.md @@ -37,6 +37,7 @@ Si quieres que revisemos tu código con una sonrisa, por favor sigue nuestras co * Ejecuta `bundle exec pronto run` y arregla los problemas de los que informe (si es que hay alguno) * Sigue [las siete reglas para un gran mensaje de commit](https://chris.beams.io/posts/git-commit/) * Añade o modifica las traducciones i18n sólo en los idiomas base, inglés (en) y español (es); todos los demás idiomas los gestionamos a través de la [integración de Crowdin](https://translate.consuldemocracy.org/). +* Dale un vistazo a nuestra [documentación sobre convenciones de código](docs/es/open_source/coding_conventions.md) para más información. Cuando revisemos tu código y te pidamos que cambies alguna cosa, si tienes experiencia con `git rebase` edita los commits existentes en vez de añadir más. Si no tienes experiencia con `git rebase`, puedes saltarte este punto. diff --git a/docs/en/SUMMARY.md b/docs/en/SUMMARY.md index eaf3ee3f6..eed0cf8be 100644 --- a/docs/en/SUMMARY.md +++ b/docs/en/SUMMARY.md @@ -55,4 +55,5 @@ * [Open Source project](open_source/open_source.md) * [Code of conduct](open_source/code_of_conduct.md) * [Contributing](open_source/contributing.md) + * [Coding conventions](open_source/coding_conventions.md) * [License](open_source/license.md) diff --git a/docs/en/customization/introduction.md b/docs/en/customization/introduction.md index 4073f91a8..95a40336b 100644 --- a/docs/en/customization/introduction.md +++ b/docs/en/customization/introduction.md @@ -55,3 +55,7 @@ So, first, make sure you've [configured your fork](../getting_started/configurat Then, if some tests fail, check one of the tests and see whether the test fails because the custom code contains a bug or because the test checks a behavior that no longer applies due to your custom changes (for example, you might modify the code so only verified users can add comments, but there might be a test checking that any user can add comments, which is the default behavior). If the test fails due to a bug in the custom code, fix it ;). If it fails due to a behavior that no longer applies, check the [tests customization](tests.md) section. We also **strongly recommend adding tests for your custom changes**, so you'll have a way to know whether these changes keep working when upgrading to a new version of Consul Democracy. + +## Coding conventions + +Consul Democracy includes linters that define code conventions for Ruby, ERB, JavaScript, SCSS and Markdown. We recommend you follow these conventions in your code as well in order to make it easier to maintain. See also the [coding conventions](../open_source/coding_conventions.md) section for more information. diff --git a/docs/en/open_source/coding_conventions.md b/docs/en/open_source/coding_conventions.md new file mode 100644 index 000000000..0f43b003c --- /dev/null +++ b/docs/en/open_source/coding_conventions.md @@ -0,0 +1,150 @@ +# Coding conventions + +## Linters + +Consul Democracy includes linters that define code conventions for Ruby, JavaScript, SCSS and Markdown. Following these conventions makes the code consistent, easier to read and easier to maintain. + +When you open a pull request, our continuous integration (CI) system automatically checks that the code in the pull request follows these conventions (you can have a look at the `.github/workflows/linters.yml` file in order to check the exact commands the CI executes). Please check the pull request report generated by these linters. + +Since you probably want to check these conventions at all times during development instead of waiting until you've already opened a pull request, we recommend you check whether your text editor has support for these linters. When an editor supports these linters, every time you save a file you'll get immediate feedback about whether you're following these conventions. + +A current limitation is that there are two Rubocop (the linter we use for Ruby) rules that aren't followed by old code and one rule that contains false positives. These rules are marked with `Severity: refactor` in the `.rubocop.yml` file; you might want to configure your editor so it ignores them. + +If your editor doesn't support these linters, you can also run the `bundle exec pronto run` command, which will only analyze the changes in the current development branch, meaning that you'll get feedback much faster than you'd do if you ran the commands to analyze every file in the project. + +## Beyond linters + +There are code conventions we follow that can't be analyzed by linters. Moreover, old code doesn't always follow these conventions, so sometimes it'll be hard to figure out which way you should proceed. + +Here are a few hints that will hopefully help. Don't try to memorize them all at once; instead, check whether these conventions are related to the code you are currently writing. + +### Only add English and Spanish texts + +Most Consul Democracy developers are fluent in English and Spanish but don't know much about other languages. On the other hand, there are many people who are fluent in other languages but don't have the technical knowledge to work with Consul Democracy's source code. That's why we use [Consul Democracy's Crowdin](https://crwd.in/consul) to manage translations for languages other than English and Spanish. + +Currently, the downside of this approach is that if you include translations for other languages in your pull request, they will be overwritten by the content in Crowdin. So don't bother including texts in other languages in your pull request; once your pull request is merged, use Crowdin to add translations to other languages instead. + +### Use components instead of views and helpers + +You might notice that HTML/ERB code appears in both `app/views/` and `app/components/`. For new code, use `app/components/`. We're gradually moving existing code from `app/views/` to `app/components/`. + +There's one scenario when you still need to add code to `app/views/`. Suppose you're creating a new controller named `AwesomeThingsController` that contains the action `index`. In this case, you need to create the `app/views/awesome_things/views/index.html.erb` file, with the following content (note that, in the code below, you might need to pass one or more parameters to the `new` method if the component needs them): + +```erb +<%= render AwesomeThings::IndexComponent.new %> +``` + +Then you would create the `app/components/awesome_things/index_component.rb` and `app/components/awesome_things/index_component.html.erb` files. + +Similarly, whenever possible, avoid adding helper methods and add methods to a component instead. Since helper methods are globally available, writing a helper method with the same name as an existing one (even if the existing one is present in a different module or even in one of our gem dependencies) will silently overwrite the existing helper. Component methods, on the other hand, are only available inside one component class, which makes them more reliable and easier to refactor. + +### Structure (S)CSS and JavaScript like components + +If you're adding some CSS or JavaScript code that only affects one component, create a file following the same structure as the component. For example, the CSS related to the code in `app/components/admin/budgets/form_component.html.erb` is located at `app/assets/stylesheets/admin/budgets/form.scss`. + +Also note that the HTML classes follow a similar structure; in this case, the `app/components/admin/budgets/form_component.html.erb` file contains an HTML element with the `budgets-form` class (note that we aren't always consistent regarding when to add an `admin-` prefix, though). + +### Use the Header component concern in new admin pages + +When writing a new page in the admin area , using the `Header` concern in components makes it easier to add a proper title and heading to the page. Define a `title` method like: + +```ruby +class Admin::AwesomeThings::IndexComponent < ApplicationComponent + include Header + + private + + def title + t("admin.awesome_things.index.title") + end +end +``` + +Then you can add `<%= header %>` to the `.html.erb` file, which will set the correct `` tag (which is very important for accessibility) as well as generating the page heading. + +### Use buttons for non-GET HTTP requests + +By default, clicking an HTML link generates a GET request. However, the `link_to` helper provided by Rails accepts a `method:` option; using it will allow you to render links that generate non-GET requests (POST, PUT/PATCH, DELETE, ...). **Do not do this**. + +Instead, use the `button_to` helper, which generates a form with a button with native support for non-GET requests. Buttons offer many advantages over links in this scenario: + +* People using screen readers will know what the button is supposed to do, but might be confused about what the link is supposed to do. +* Buttons work when JavaScript hasn't loaded or is disabled. +* Most browsers allow opening links in a new tab; for non-GET requests, that will usually generate an error. + +Similarly, use buttons instead of links for controls that don't generate any HTTP requests but modify the content of the page using JavaScript (for example, to hide or show certain content). + +### Use Tenant secrets instead of Rails secrets + +Rails manages confidential information by storing it in the `config/secrets.yml` file. Usually, Rails applications access the contents of this file using the `Rails.application.secrets` method. + +Consul Democracy, however, is a [multitenant application](../features/multitenancy.md). With a few exceptions, most secrets allow different values for different tenants. In this case, using `Tenant.current_secrets` instead of `Rails.application.secrets` will correctly find different values depending on the current tenant. + +### Browser support + +We try to support 100% of the browsers whenever possible. That means that we don't use ECMAScript 2015 (or later) but use the ECMAScript 5 syntax instead. + +However, sometimes, particularly when styling with CSS, supporting 100% of the browsers isn't feasible. In this case, we aim to make the page look as expected on browser versions that are 7 years old or newer (about 98% of the browsers out there), while making sure things don't look too broken everywhere else. + +### Right-To-Left support + +In order to support Right-To-Left (RTL) languages, like Arabic or Hebrew, when writing CSS, use properties like `margin-#{$global-left}` or `margin-#{$global-right}` instead of `margin-left` or `margin-right`. Same with padding and other positional properties. The `$global-left` variable is set to `left` on languages written from left to right and to `right` on languages written from right to left; the `$global-right` variable takes the opposite value. + +Until there's universal browser support for them, don't use logical properties like `margin-inline`. + +### Use rem units instead of rem-calc + +In our (S)CSS code, you'll probably find many places using Foundation's `rem-calc` function to convert pixels to rems. Other places directly use `rem` to define rems. At some point, we'll remove every usage of `rem-calc`, so use `rem` instead. + +### Write component tests for scenarios with no user interaction + +For years, we almost exclusively wrote system tests to check the content displayed on a page. However, system tests are very slow and, when Consul Democracy started to grow, our test suite reached a point where it takes too long to run it on a development machine and we need to rely on a continuous integration system. To prevent the test suite from becoming even slower, only write system tests when testing some kind of user interaction like clicking links or filling in forms. To test that, for example, certain content is rendered for verified users but not rendered for unverified ones, write a component test instead. + +### Don't check the database after a system test + +In general, system tests have four parts: + +1. Setting up the database +2. Starting the browser using the `visit` method +3. Doing some kind of interaction +4. Checking the results of the interaction + +When checking the results of the interaction, don't check the content of the database (for example, by checking `Proposal.count` after creating a proposal) because it might result in a flaky database exception when the process running the test and the process that started the browser both access the database. Instead, check the content of the page as seen from the user's point of view (for example, check the number of proposals appearing on the proposals index page instead of checking `Proposal.count`). + +Checking the database instead of checking the user's point of view can also lead to usability and accessibility issues. For example, a button which modifies the database without giving users any feedback is a usability issue that will be detected when checking the user's point of view. For the same reason, you should try to use reference texts instead of HTML attributes; for example, instead of writing `fill_in "residence_document_number", with: "12345678Z"`, you should write `fill_in "Document number", with: "12345678Z"`. The latter checks that a label is correctly associated with the input, while the former does not. + +### Avoid simultaneous requests in system tests + +When using methods like `click_link`, make sure the request has finished before generating a new one. Getting this point right is really hard (sometimes we don't do it right either), so we really appreciate your efforts regarding this point. + +Here's an example: + +```ruby +scenario "maintains the navigation link" do + visit admin_root_path + click_link "Proposals" + + within("#side_menu") { expect(page).to have_link "Proposals" } +end +``` + +The problem of this test is that the expectation is also true without clicking the "Proposals" link. When writing this code, we probably want the browser to click on the "Proposals" link, wait for the request to finish, and then check the content of the page. That is not what's happening here, though. + +Instead, what's happening is: the browser clicks on the link and the test immediately checks the content of the page. If the expectation is met, the test doesn't wait for the request to finish. That means that the request might finish when a different test has started and, after that, anything can happen. For example, the data from the two different tests might get mixed up. + +This sometimes results in a so-called "flaky spec", which is a test that fails sometimes but not 100% of the time. Flaky specs are really harmful because, when facing a test suite with a failure during a pull request, you no longer know the cause of the failure, which might not even be related to the pull request. + +The example above would be solved with: + +```ruby +scenario "maintains the navigation link" do + visit admin_root_path + click_link "Proposals" + + within("#side_menu") { expect(page).to have_css "[aria-current]", exact_text: "Proposals" } +end +``` + +This time, the expectation isn't true before clicking the "Proposals" link, so the test waits until the request has finished. + +By the way, you might notice that here we're checking an HTML attribute, which seems to be the opposite of what we recommended in the [don't check the database after a system test](#dont-check-the-database-after-a-system-test) section. However, people using screen readers will be notified about the `aria-current` attribute, so we're actually testing the page from the user's point of view. diff --git a/docs/en/open_source/contributing.md b/docs/en/open_source/contributing.md index 9e2fa6bc7..dbde2749f 100644 --- a/docs/en/open_source/contributing.md +++ b/docs/en/open_source/contributing.md @@ -23,6 +23,7 @@ We suggest to follow these steps to keep a good track of the changes you're abou - First of all, add a comment to the issue to make everyone know you are going to work on it. If the issue has someone assigned it means that person is already solving it. - Fork the project. - Create a feature branch based on the `master` branch. To make it easier to identify, you can name it with the issue number followed by a concise and descriptive name (e.g. `123-fix_proposals_link`). +- Check our [coding conventions](coding_conventions.md) to help you decide how to write your code. - Work in your branch committing there your changes. - Make sure all tests are passing. In case you're extending or creating a new feature, consider adding its own specs. - Once you've finished, send a **pull request** to the [Consul Democracy repository](https://github.com/consuldemocracy/consuldemocracy/) describing your solution to help us understand it. It's also important to tell what issue you're addressing, so specify it in the pull request description's first line (e.g. `Fixes #123`). diff --git a/docs/en/open_source/open_source.md b/docs/en/open_source/open_source.md index 291a7a35f..c1a493a8f 100644 --- a/docs/en/open_source/open_source.md +++ b/docs/en/open_source/open_source.md @@ -2,3 +2,4 @@ * [Code of conduct](code_of_conduct.md) * [Contributing](contributing.md) +* [Coding conventions](coding_conventions.md) diff --git a/docs/es/SUMMARY.md b/docs/es/SUMMARY.md index 6131aeac4..40702aa0c 100644 --- a/docs/es/SUMMARY.md +++ b/docs/es/SUMMARY.md @@ -55,4 +55,5 @@ * [Proyecto Open Source](open_source/open_source.md) * [Código de conducta](open_source/code_of_conduct.md) * [Contribuciones](open_source/contributing.md) + * [Convenciones de código](open_source/coding_conventions.md) * [Licencia](open_source/license.md) diff --git a/docs/es/customization/introduction.md b/docs/es/customization/introduction.md index f5e4292ae..ca66da73a 100644 --- a/docs/es/customization/introduction.md +++ b/docs/es/customization/introduction.md @@ -55,3 +55,7 @@ Así que, en primer lugar, asegúrate de haber [configurado tu "fork"](../gettin En caso de que alguno de los tests falle, echa un vistazo a uno de los tests y comprueba si falla por un fallo en el código personalizado o porque el test comprueba un comportamiento que ha cambiado con los cambios personalizados (por ejemplo, puede que modifiques el código para que solamente los usuarios verificados puedan añadir comentarios, pero puede que haya un test que compruebe que cualquier usuario puede añadir comentarios, ya que es el comportamiento por defecto). Si el test falla debido a un fallo en el código personalizado, arréglalo ;). Si falla debido a un comportamiento que ha cambiado, consulta la sección de [personalización de tests](tests.md). **Recomendamos encarecidamente añadir tests a tus cambios personalizados** para que tengas una forma de comprobar si estos cambios siguen funcionando cuando actualices a una nueva versión de Consul Democracy. + +## Convenciones de código + +Consul Democracy incluye herramientas (_linters_) para definir convenciones de código Ruby, ERB, JavaScript, SCSS y Markdown. Te recomendamos seguir estas mismas convenciones en tu código para que sea más fácil de mantener. Échale un vistazo a la sección de [convenciones de código](../open_source/coding_conventions.md) para más información. diff --git a/docs/es/open_source/coding_conventions.md b/docs/es/open_source/coding_conventions.md new file mode 100644 index 000000000..dfe618303 --- /dev/null +++ b/docs/es/open_source/coding_conventions.md @@ -0,0 +1,150 @@ +# Convenciones de código + +## Linters + +Consul Democracy incluye unas herramientas llamadas _linters_ que definen convenciones de código para Ruby, JavaScript, SCSS y Markdown. Seguir estas convenciones hace que el código sea consistente y más fácil de leer y mantener. + +Cuando abras una _pull request_ (PR), nuestro sistema de integración continua (CI, por sus siglas en inglés) automáticamente comprueba que el código de la PR sigue estas convenciones (puedes ver el fichero `.github/workflows/linters.yml` para comprobar exactamente qué órdenes se ejecutan). Por favor, comprueba el informe generado por estos linters que aparecerá en la PR. + +Es muy probable que prefieras seguir estas convenciones mientras desarrollas en vez de tener que esperar a tenerlo todo listo para abrir una PR. Para ello, recomendamos que compruebes si tu editor ofrece soporte para estos linters. Cuando un editor tiene suporte para estos linters, cada vez que grabas un fichero recibes información de forma inmediata sobre si estás siguiendo estas convenciones. + +Una limitación que tenemos en la actualidad es que hay dos reglas de Rubocop (el linter que usamos para Ruby) que no se siguen siempre en código escrito hace varios años y otra regla que da falsos positivos. Estas reglas aparecen marcadas con `Severity: refactor` en el fichero `.rubocop.yml`; sería conveniente que configuraras tu editor para que no informase sobre estas reglas. + +Si tu editor no ofrece soporte para estos linters, puedes ejecutar la orden `bundle exec pronto run`, que analizará los cambios que hay en tu rama actual, con lo que es considerablemente más rápido que ejecutar las órdenes que analizan todos los ficheros del proyecto. + +## Más allá de los linters + +Los linters no pueden comprobar todas las convenciones de código que seguimos. Además, hay código antiguo que no sigue estas convenciones, con lo que a veces te será difícil saber qué camino tomar. + +A continuación, comentamos algunas claves que esperamos te sirvan de ayuda. No intentes memorizarlas todas de golpe; en lugar de ello, comprueba cuáles de estas convenciones están relacionadas con el código que que estás escribiendo ahora mismo. + +### Añade solamente textos en inglés y en español + +La mayoría de desarrolladores de Consul Democracy hablan inglés y español pero su conocimiento de otros idiomas es bastante limitado. Por otro lado, hay mucha gente que sabe otros idiomas pero no tiene el conocimiento técnico suficiente como para trabajar con el código fuente de Consul Democracy. Por eso, utilizamos el [Crowdin de Consul Democracy](https://crwd.in/consul) para las traducciones a otros idiomas. + +Actualmente, esta configuración tiene un inconveniente, y es que si abres una _pull request_ (PR) que incluya cambios en otros idiomas, estos cambios serán reemplazados por el contenido de Crowdin. Así que es mejor que no incluyas textos en otros idiomas en una PR; en lugar de esto, una vez que el contenido de la PR haya sido incluido en Consul Democracy, utiliza Crowdin para añadir traducciones a otros idiomas. + +### Usa componentes en vez de vistas y "helpers" + +Es posible que hayas visto código HTML/ERB tanto en el directorio `app/views/` como en el directorio `app/components/`. Para código nuevo, utiliza `app/components/`. Estamos moviendo código existente de `app/views/` a `app/components/` paulatinamente. + +Solamente hay un caso en que tienes que añadir código en `app/views/`. Supongamos que estás añadiendo un controlador nuevo llamado `AwesomeThingsController` que contiene la acción `index`. En este caso, necesitammos crear el archivo `app/views/awesome_things/views/index.html.erb`, con el siguiente contenido (nótese que, en el código que aparece a continuación, es posible que tengas que pasar uno o más parámetros al método `new` si el componente los necesita): + +```erb +<%= render AwesomeThings::IndexComponent.new %> +``` + +El código al que se llama desde aquí iría en los archivos `app/components/awesome_things/index_component.rb` y `app/components/awesome_things/index_component.html.erb`. + +De manera similar, cuando sea posible, evita usar métodos de _helpers_ y en su lugar añade métodos a los componentes. Como los métodos de _helpers_ están disponibles de forma global, al escribir un método de _helper_ con el mismo nombre que uno existente (incluso si el método existente está en otro módulo o incluso en una de nuestras dependencias), el método nuevo reemplazará al antiguo sin informar siquiera de ello. Los métodos de componentes, en cambio, solamente pueden accederse desde la clase del componente, lo que los hace más fiables y fáciles de refactorizar. + +### Sigue la misma estructura de ficheros (S)CSS y JavaScript que para componentes + +Si añades código de CSS o JavaScript que solamente afecta a un componente, crea un archivo siguiente la misma estructura de ficheros que dicho componente. Por ejemplo, el CSS relacionado con el código en `app/components/admin/budgets/form_component.html.erb` está en `app/assets/stylesheets/admin/budgets/form.scss`. + +Las clases de HTML también siguen una estructura similar; por ejemplo, el archivo `app/components/admin/budgets/form_component.html.erb` contiene un elemento HTML con la clase `budgets-form` (nótese que, sin embargo, no siempre somos consistentes en cuanto a cuándo añadir el prefijo `admin-`). + +### Utiliza el "concern" Header en páginas del área de administración + +Cuando escribas una nueva página en el área de administración, utilizar el _concern_ `Header` en componentes hace que sea más fácil añadir un título y un encabezado adecuado a la página. Añade un método `title` como: + +```ruby +class Admin::AwesomeThings::IndexComponent < ApplicationComponent + include Header + + private + + def title + t("admin.awesome_things.index.title") + end +end +``` + +Esto permite añadir `<%= header %>` en el archivo `.html.erb`, que hará que se incluya la etiqueta `<title>` con el texto adecuado (lo cual es muy importante para temas de accesibilidad) y generará el encabezado de la página. + +### Usa botones para peticiones HTTP que no sean GET + +Por defecto, pinchar un enlace en un navegador genera una petición GET. Sin embargo, el método `link_to` de Rails acepta una opción `method:`; usarla permite incluir enlaces que generan otro tipo de peticiones (POST, PUT/PATCH, DELETE, ...). **No lo hagas**. + +En lugar de esto, utiliza el método `button_to`, que genera un formulario con un botón con soporte nativo para otras peticiones. Usar botones en esta situación tiene varias ventajas: + +* Las personas que usan lectores de pantalla sabrán qué va a pasar al pinchar en el botón, pero no tendrán claro qué pasará al pinchar en el enlace. +* Los botones funcionan cuando JavaScipt no ha cargado o está deshabilitado. +* La mayoría de navegadores permiten abrir un enlace en una nueva pestaña; para peticiones que no sean GET, normalmente esto generará un error. + +Por la misma razón, utiliza botones en lugar de enlaces para controles que no generan peticiones HTTP sino que modifican el contenido de la página usando JavaScript (por ejemplo, para mostrar u ocultar cierto contenido). + +### Utiliza secretos de entidades en vez de secretos de Rails + +Rails gestiona la información confidencial mediante el archivo `config/secrets.yml`. Normalmente, las aplicaciones de Rails acceden a los contenidos de este fichero utilizando el método `Rails.application.secrets`. + +Consul Democracy, sin embargo, es una [aplicación mulitentidad](../features/multitenancy.md). Con pocas excepciones, la mayoría de los secretos definidos en `config/secrets.yml` permiten distintos valores para distintas entidades. En este caso, utilizar `Tenant.current_secrets` en lugar de `Rails.application.secrets` encontrará el valor correcto para la entidad actual. + +### Soporte para navegadores + +Intentamos dar soporte al 100% de los navegadores siempre que sea posible. Eso quiere decir que no utilizamos ECMAScript 2015 (o posterior) sino que usamos la sintaxis de ECMAScript 5. + +Sin embargo, en ocasiones, especialmente al añadir estilos con CSS, no es viable usar estilos que funcionen en el 100% de los navegadores. En este caso, el objetivo es hacer que la página se vea tal y como se espera en versiones de navegadores que tengan 7 años o menos (aproximadamente el 98% de los navegadores que usa la gente) a la vez que nos aseguramos de que la página no se vea excesivamente mal en el resto de navegadores. + +### Soporte para idiomas que se escriben de derecha a izquierda + +Para la correcta visualización en idiomas que se escriben de derecha a izquierda (RTL, por sus siglas en inglés), como árabe o hebreo, al escribir CSS, utiliza propiedades como `margin-#{$global-left}` or `margin-#{$global-right}` en lugar de `margin-left` o `margin-right`. Lo mismo con las propiedades de _padding_ u otras propiedades que definen posiciones. La variable `$global-left` tiene el valor `left` (izquierda) en idiomas que se escriben de izquierda a derecha y `right` (derecha) en idiomas que se escriben de derecha a izquierda; la variable `$global-right` toma el valor opuesto a `$global-left`. + +Hasta que haya soporte universal en navegadores, no uses propiedades lógicas como `margin-inline`. + +### Utiliza unidades rem en lugar de rem-calc + +En nuestro código (S)CSS, seguramente encuentres muchos sitios que usan la función `rem-calc` de Foundation para convertir píxeles a rems. Otros sitios usan `rem` directamente para definir rems. Tarde o temprano acabaremos quitando los usos existentes de `rem-calc` así que utiliza `rem` en código nuevo. + +### Escribe tests de componentes para escenarios donde no hay interacción + +Durante años, estuvimos escribiendo tests de sistema para comprobar el contenido que aparece en una página. Sin embargo, los tests de sistema son muy lentos y, cuando Consul Democracy empezó a crecer, llegamos al punto en que ejecutar todos nuestros tests en una máquina de desarrollo tarda demasiado y dependemos de un sistema de integración continua. Para evitar que los tests tarden aún más, escribe tests de sistema solamente para probar qué sucede cuando el usuario interactúa con la página, como pinchando en enlaces o rellenando formularios. Para probar que, por ejemplo, hay contenido que se renderiza para usuarios verificados pero no para usuarios no verificados, escribe un test de componente. + +### No compruebes la base de datos tras un test de sistema + +En general, los tests de sistema tienen cuatro partes: + +1. Introducir en la base de datos los datos necesarios para el test +2. Arrancar el navegador usando el método `visit` +3. Interactuar con la página de alguna forma +4. Comprobar los resultados de dicha interacción + +Cuando compruebes el resultado de la interacción, no compruebes el contenido de la base de datos (por ejemplo, comprobando el resultado de `Proposal.count` después de crear una propuesta) porque podría resultar en una excepción inconsistente de base de datos cuando tanto el proceso que ejecuta los tests como el proceso que arrancó el navegador acceden a la base de datos. En lugar de esto, comprueba que el contenido de la página desde el punto de vista del usuario (por ejemplo, comprueba el número de propuestas que aparecen en el índice de propuestas en vez de comprobar `Proposal.count`). + +Comprobar la base de datos en lugar de comprobar el punto de vista del usuario puede llevar a problemas de usabilidad y accesibilidad. Por ejemplo, un botón que modifique la base de datos pero aparentemente no haga nada desde el punto de vista del usuario es un problema de usabilidad que puede detectarse escribiendo tests desde el punto de vista del usuario. Por la misma razón, deberías utilizar textos que aparecen en pantalla en vez de atributos HTML; por ejemplo, en lugar de escribir `fill_in "residence_document_number", with: "12345678Z"`, escribe `fill_in "Document number", with: "12345678Z"`. Esto último comprueba que hay una etiqueta ("label") asociada correctamente con ese campo del formulario, mientras que usar el atributo HTML no lo hace. + +### Evita peticiones simultáneas durante tests de sistema + +Al usar métodos como `click_link`, asegúrate de que la petición ha terminado antes de generar otra. Conseguir esto en todo momento es muy difícil (nosotros también nos equivocamos en esto a veces), con lo que agradecemos enormemente que tengas esto en mente. + +Como ejemplo, este test: + +```ruby +scenario "maintains the navigation link" do + visit admin_root_path + click_link "Proposals" + + within("#side_menu") { expect(page).to have_link "Proposals" } +end +``` + +El problema de este test es que la expectativa también se cumple antes de pinchar en el enlace "Proposals". Con este test, lo que probablemente queramos sea que el navegador pinche en el enlace "Propuestas", espere a que la petición termine, y que después el test compruebe el contenido de la página. Sin embargo, eso no es lo que está pasando. + +Lo que está pasando es que el navegador pincha en el enlace e inmediatamente el test comprueba el contenido de la página. Si la expectativa se cumple, el test no espera a que la petición termine. Eso quiere decir que la petición puede terminar cuando ya se esté ejecutando otro test distinto y, cuando eso sucede, puede pasar cualquier cosa. Por ejemplo, que se mezclen datos de dos tests diferentes. + +Esto a veces resulta en tests que fallan de manera inconsistente (en inglés esto se conoce como "flaky test" o "flaky spec"), es decir, que fallan a veces, pero no siempre. Este tipo de tests supone un gran inconveniente, puesto que, si falla un test durante una _pull request_, vas a tardar un rato en averiguar si el test falla por cambios relacionados con esa _pull request_ o no. + +El ejemplo anterior se resolvería con: + +```ruby +scenario "maintains the navigation link" do + visit admin_root_path + click_link "Proposals" + + within("#side_menu") { expect(page).to have_css "[aria-current]", exact_text: "Proposals" } +end +``` + +En este caso, la expectativa no se cumple antes de pinchar en el enlace "Proposals", con lo que el test espera a que la petición termine. + +Por cierto, es posible que hayas notado que en este test estamos comprobando un atributo HTML, que parece justo lo contrario de lo que recomendábamos en la sección [no compruebes la base de datos tras un test de sistema](#no-compruebes-la-base-de-datos-tras-un-test-de-sistema). Sin embargo, la gente que usa lectores de pantalla sí que oirá la información acerca del atributo `aria-current`, con lo que estamos probando el test desde el punto de vista del usuario. diff --git a/docs/es/open_source/contributing.md b/docs/es/open_source/contributing.md index 76d284b4c..b0dd4dca7 100644 --- a/docs/es/open_source/contributing.md +++ b/docs/es/open_source/contributing.md @@ -23,6 +23,7 @@ Te sugerimos seguir los siguientes pasos para facilitar el seguimiento de los ca - Primero, añade un comentario en el issue para notificar que vas resolverlo. Si el issue tiene a alguien asignado significa que ya hay alguien encargado de él. - Crea un fork del proyecto. - Crea una rama de funcionalidad basada en la rama `master`. Para identificarla más fácilmente, puedes nombrarla con el número del issue seguido de un nombre conciso y descriptivo (por ejemplo: `123-fix_proposals_link`). +- Comprueba nuestras [convenciones de código](coding_conventions.md) para ayudarte a decidir cómo escribir tu código. - Desarrolla los cambios haciendo commits en tu nueva rama. - Asegúrate de que todos los tests pasan. Si estás extendiendo una funcionalidad o creando una nueva, considera añadir sus propios tests. - Cuando hayas terminado, envía un **pull request** al [repositorio de Consul Democracy](https://github.com/consuldemocracy/consuldemocracy/) describiendo la solución que propones para ayudarnos a entenderlo. También es importante que especifiques qué issue estás resolviendo al principio de la descripción del PR (por ejemplo, `Fixes #123`). diff --git a/docs/es/open_source/open_source.md b/docs/es/open_source/open_source.md index 1c50259a0..b2e9b9820 100644 --- a/docs/es/open_source/open_source.md +++ b/docs/es/open_source/open_source.md @@ -2,3 +2,4 @@ * [Código de conducta](code_of_conduct.md) * [Contribuciones](contributing.md) +* [Convenciones de código](coding_conventions.md)