Skip to content
David Cook edited this page Mar 4, 2024 · 19 revisions

Firstly, code conventions are enforced by Rubocop, in order to improve code consistency and quality. Please ensure you check with Rubocop before each commit (consider adding it to a pre-commit hook).

This pages lists additional conventions that are not verified by Rubocop, but we agree to follow.

Ruby/Rails

Use existing code convention patterns where appropriate. Including:

Queries

Please adhere to the naming convention when introducing new query-related services in the app/queries folder. This consistency will streamline code readability and maintain a clear understanding of the purpose of each service.

Database Migrations

See this guide: Database-migrations

HTML Templates

Use j when inserting an arbitrary Ruby string in a JS or Angular expression

j is an alias for escape_javascript.

When you have to insert an arbitrary Ruby string to a Javascript or Angular expression, this will ensure that characters are properly escaped and will not cause parsing issues or syntax errors.

# Bad
# JS syntax error when description contains a single-quote
"{description: '#{enterprise.description}'}"

# Good
# Single-quote is escaped
"{description: '#{j enterprise.description}'}"

Tests

Prefer plain text over method calls in expected values

For example, when asserting text that is translatable, use plain text instead of the I18n.t call.

Bad:

expect(json_response['errors']).to eq I18n.t('admin.order_cycles.bulk_update.no_data')`

# Does not catch typos in translation values, especially when including variables.
# Example: "Welcome #{user.name}" is wrong while "Welcome %{user.name}" is right.
#
# Does not catch duplicate translation keys in the locale. If you accidentally define
# a key twice in a YAML file then only the second one is applied. If you wanted the
# first translation defined in the file then the test passes while the app prints the
# wrong text.

Good:

expect(json_response['errors']).to eq "Hm, something went wrong. No order cycle data found."

This format is much easier to read. It's also more likely to find typos and bugs this way. You still need to update a spec when changing the text but that is a user-facing change. In return, you don't need to update the spec when you move or rename translation keys.

Read more about the discussion

Performance

Avoid N+1 queries

Eager loading attributes

Example 1

In app/serializers/api/address_serializer.rb we want to add the country to the address so we do:

def country_name
    object.country.andand.name
end

If address.country is not eager loaded, this code will trigger a N+1 query.

TODO: how to avoid this

Example 1

Using has_many declarations in serializers is one of the most common causes of N+1 queries in code.

TODO: how to avoid this

Clone this wiki locally