Pager plugins do not validate input

Triaged as part of BSI

A few of us agreed this needs some more actionable points. This seems like more of a theoretical thing that could lead to bugs? Also the mentioned issue is now closed. Should this be a task?

🌱 [meta] Add constraints to all config entity types Active only talks about the Views entity config, but ideally we should also implement constraints for all Views plugins, so repurposing this for that

Hello @bbrala
Your MR looks good.
But, I have one query. Currently, in Views configuration file, if the `offset` or `items_per_page` values are provided as strings (even though they should be integers), the configuration import (cim) works fine without any errors. However, when I open the view in the UI, these values appear as 0.

Ideally, the system should throw an error during the config import itself, indicating that `offset` or `items_per_page` must be integers.

So, moving this issue to Needs Work.

Thanks.

I tried to reproduce this, but eve if i import with a string it ends up as integer and i can do changes.

Can you make a better reproduce step so i can see it locally?

@bbrala Yes, you are right and you reproduced it correctly, that if we import it as a string, it ends up as interger, ie - 0.
But i believe it would be better if we give an error message while importing, something like - String passed, expects integer value.

Currently the validation is not enabled from the config import event. See 📌 Validate configuration schema before importing configuration Needs work . So your usecase is currently not supported. We can validate in the linked issue, but that would also now need to adjust to validate when we have a fully compatible and well defined schema for the config in question. Right now not all config is validatable yet, which would probably mean havoc everywhere.

You say you get -0 as a result, or just 0? What is the value in the config you have? Since -0 does feel impossible to set through UI.

items_per_page and pagination_heading_level are removed from the views_pager_sql schema, but I don't see a reason for that in the issue, can you help me understand this?

Updated the page title to be in line with most of the other issues in the validation space.

The parent type has these fields.

views_pager_sql:
  type: views_pager

views_pager has and validated those fields. The views_pager_sql extends the earlier type and adds more, but also still has the existing fields afaik.

Ok, I double check that as well, makes sense. All my questions have now been answered.

Hm, over in 📌 Add validation constraints to contact.settings Active we just committed a bunch of changes with

      constraints:
        Range:
          min: 0

Is PositiveOrZero preferred? If so, why? There is no discussion of adding it in this issue. We should be consistent, in which case should we update other cases of Range with a minimum of 0?

Actually we don't have any rules about this, but I think I prefer PositiveOrZero, just because it's more expressive compared to Range: min: 0.

Added 📌 [pp-1] Update use of Range constraint with min 0 to use PositiveOrZero instead Postponed as a follow-up to move other cases of Range w/ min 0 to PositiveOrZero - I agree with @borisson_ - it is more expressive

Credits

Committed to 11.x and unpostponed the follow-up - thanks all

larowlan → closed merge request !11809

Automatically closed - issue fixed for 2 weeks with no activity.