-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow setting 'isSortable' on Schema generated fields not only smart fields #309
Comments
Hi @yoadsn, If I read well, you want to have the opportunity to override the "native" fields default configuration using This is not the current purpose of If you want to deactivate the sorting, according to the ForestAdmin vision, it should be done from the UI configuration panels. With this in mind, I would consider this issue as a feature request to enable sorting activation/deactivation on specific fields. What do you think about this idea? |
Thanks you @arnaudbesnier for clarifying that - I see this in a slightly different light. Since I have been working for a while with ForestAdmin, I have come to realize the limitations of setting ui logic (or sometimes it is business logic) from the Forest layouts configuration. Generally speakingThe fact that such rules cannot be maintained and tracked as part of the source code is a big flaw in dev driven processes. The "layouts" concept today from a developers' point of view is vague, opaque an inflexible. Also, copying between envs and across teams has some existing bugs and inconsistent behaviors. Our entire CI/CD process is automated - but still the layouts have to be manually done when we deploy - in 2020 this is insane. About sorting as a UI decisionAllowing to "sort" is not only a decision of the UI but rather a decision of the model designer first. The store has to be properly setup to allow sorting in terms of performance before allowing such an option in the UI. Only then can a layout designer decide to turn it off (if available at all). About the design concepts of LianaAlthough the Consider things like array of enums, which exists in mongoose but not supported in Liana today (it just creates a simple string array in the liana schema) - this could be a valid point of extension assuming mongoose has a problematic support. Even "hooks" which mongoose supports may not cover all use cases of aspect decorations. I look at the mongoose (or sequalize) schema as a way to define a Liana model, but not the only way to define it - in my opinion you should move liana-express in a direction where a developer could develop it's own "liana-X-express' layer according to his logic if needed but adding extension points are a natural part of that vision for cases where the DB model is a close-enough starting point. What do you think? Would love to discuss more over slack if it helps. |
Ok, For your CI/CD needs did you ever consider using our toolbelt? I agree with your thought about the sorting. About your last point, the full liana implementation is open-sourced and not such a complex layer, feel free to fork the codebase and suggest improvements. |
@arnaudbesnier Thanks for being open! Yes, we have attempted using the toolbelt but last we checked it did not support the new schema structure so it fails with the new Liana. (Reported that to Louis about 1m ago). But even when it works, the layout copy is always all-or-nothing, merging, when working with multiple developers is eventually going to be needed - I can 100% see how complex it would be to support a code-based layout and did not expect that to happen - just pointing out that if we had all the resources in the world (😍) the architecture should lean towards that. Regarding the sorting - no worries! there are many workarounds (like overriding the path and filtering out unwanted sorts). Filddeling with generated code is where I draw the line for my team - this usually just leads to bigger problems down the road. Would you accept PR's on the core Liana implementation? (or the mongoose implementation)? BTW.. Forest is AWESOME! |
Expected behavior
When specifying the name of a field on the "collection" configurator (usually used for Smart Fields) with the "isSortable" property - it would use that value and not the default "false".
Actual behavior
The field is duplicated in the schema.json file and as a result in the UI as well (well, it basically generates a smart field with the same name as the native field)
Context
We are using mongoose - but this seems to be relevant to any Liana implementations.
At the core of the issue, the need to set field properties for fields which are derived from the ORM schema. Some properties like "isRrequired" can be inferred from the DB schema definition, others like "isSortable" are not.
Still, for performance reasons we would like to limit which fields are available for sorting by.
Ideally, if we use the
forest-express
modulescollection
export - the one that is used for smart fields and smart collections etc., we could just specify the name of a db derived field with theisSortable
value set tofalse
which would override the defaulttrue
.This is not working since:
here the collection configurator concats field defs instead of possibly merging them.
The formatter for the field options here does his job by parsing the value correctly.
Of course, conflicts nay arise, so I would gate those "extensions" to native fields with only valid settable attributes and a clear override priority.
The existing field duplication behaviour to me seems like a bug - either throw on such cases blocking this option or properly implement a merge.
Hope this helps!
The text was updated successfully, but these errors were encountered: