Redesign metadata and Query Builder #31
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a module uses `Trans`, two underscore functions will be added to provide the necessary metadata for the `Trans.QueryBuilder` and `Trans.Translator` functions. This underscore functions are: - `__trans__(:container)`, which returns the name of the translation container field. - `__trans__(:fields)`, which returns a list of the translatable fields.
The translation container customization allowed users to declare an alternative field (the default was `:translations`) as the field that contained the translations for the schema. This customization has been sacrificed to ease the rewrite of the QueryBuilder module. Now, Trans will always look in the `:translations` field and there is no way to customize it.
All the functions of the `Trans.QueryBuilder` module have been removed and replaced with the macro `translated/2`. This macro generates an SQL Fragment and can be directly used from inside an `Ecto.Query`. This removes the necessity of building the query before calling the `Trans.QueryBuilder` functions and integrates seamelessly with `Ecto.Query` provided functions for comparisons, etc. This also permits adding conditions on joined schemas.
The QueryBuilder tests have been updated to use the newly introduced `translated` macro.
The translator module does not require to be passed the name of the translation container field. The test have also been updated to reflect this change.
crbelaus
force-pushed
the
issues/28-redesign-query-builder
branch
2 times, most recently
from
1c4cdb8
to
70e3e2b
Compare
In PostgreSQL 9.4 [1] the operators had different precedence rules than in PostgreSQL 9.5 or higher [2] (take a look at "any other operator" in the table. This differences caused errors when triggering a query that contained something similar to `where a->b is not null`, which 9.4 would interpret as `where a->(b is not null)` and 9.5 or higher would interpret as `where (a->b) is not null`. This commits adds parentheses around the JSONB accesses to avoid this kind of problems by stating the desired preference explicitly. [1] https://www.postgresql.org/docs/9.4/static/sql-syntax-lexical.html [2] https://www.postgresql.org/docs/9.5/static/sql-syntax-lexical.html
crbelaus
force-pushed
the
issues/28-redesign-query-builder
branch
from
70e3e2b
to
e8d0d20
Compare
Options provided when using `Trans` are now stored into a module attribute.
`Trans` will validate the fields declared as translatable and raise an error during compilation time if they are not declared in the module's struct or schema definition.
crbelaus
changed the title
Trans.Translator now validates that whether the field is translatable and provides a useful error message instead of the default `KeyError`. Docs have been updated and typespecs added.
The function `Trans.translatable?/2` checks whether a field is translatable or not.
crbelaus
changed the title
The macro `Trans.QueryBuilder.translated/3` now validates the translated fields before the macro expansion step. We will receive an error in compilation time when adding conditions on untranslatable fields.
crbelaus
force-pushed
the
issues/28-redesign-query-builder
branch
from
0549fcf
to
cbb6ba1
Compare
|
Ready to be merged 🚀 |
This was referenced Apr 2, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Trans.QueryBuildermodule is one of the main selling points of Trans. It allows easy generation of queries with conditions on translated fields, avoiding the need for joining tables or writing SQL fragments.The
Trans.QueryBuildermodule uses a very naive (you may even say flawed) approach for generating and refining queries. Basically it must receive anEcto.Queryablewhich will then be refined with extra conditions. This has two main flaws:Trans.QueryBuildermodule. Ideally, we want to state the conditions directly when building the query. This way we can use the functions already provided byEcto.Queryfor comparisons, etc.Trans.QueryBuildercan only add conditions to the first schema selected in theEcto.Queryablethat it receives. If you want to add conditions to a joined schema, you are out of luck.This PR fixes those mistakes by rewriting the
TransandTrans.QueryBuildermodules. The changes are:Transit will only obtain a__trans__/1function, which provides the metadata required for Trans modules. This avoids adding extra concerns and keeps the schema modules clean.Transwill raise an error and abort the compilation.Trans.QueryBuilderhave been replaced by theTrans.QueryBuilder.translated/2macro. This macro can be used directly when building anEcto.Queryand generates an SQL fragment that interoperates seamlessly with the rest of functions provided byEcto.Query.translationsfield. I plan to add this functionality back after this update has been released and tested properly.To Do
translated/2macro and will be used for validating if the required field is among the translatable fields.