Skip to content
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

Make dropdown of error codes configurable in yaml #90

Closed
andrewandante opened this issue Sep 25, 2023 · 4 comments · Fixed by #91
Closed

Make dropdown of error codes configurable in yaml #90

andrewandante opened this issue Sep 25, 2023 · 4 comments · Fixed by #91

Comments

@andrewandante
Copy link
Contributor

andrewandante commented Sep 25, 2023

Spawned as a result of #89

There are a lot of valid error codes, and for the sake of completionism it makes sense to have them all available from the dropdown. However, given these are available for content authors to use, it also makes sense to limit the default set.

It'd be great to move the list of codes to configuration, and set the default list to something like:

  • 400 Bad Request
  • 401 Unauthorized
  • 403 Forbidden
  • 404 Not Found
  • 500 Server Error

With the option to include all or customise the list that you'd like the authors to see.

PR

@GuySartorelli
Copy link
Member

GuySartorelli commented Sep 25, 2023

Moving my thoughts here:

I probably wouldn't limit it by default (some people might consider that a breaking change... it's a grey area as to whether it would be or not) - rather I'd include the full list in the default set, and developers can choose to hide any they want to by setting them to null

e.g. if it was defined as

private static array $error_codes = [
  '404' => 'not found',
  '418' => 'teapot',
  // etc
];

Then developers could add this to their yaml

SilverStripe\ErrorPage\ErrorPage:
  error_codes:
    418: null

Alternatively, we could have a separate disallowed_error_codes config, or a allowed_error_codes config that is null by default but if it's an array, it overrides the default full set.

@GuySartorelli GuySartorelli changed the title ENH Make dropdown of codes configurable in yaml Make dropdown of error codes configurable in yaml Sep 25, 2023
@andrewandante
Copy link
Contributor Author

some people might consider that a breaking change

Ah that's a good point, I hadn't considered that.

I think I prefer the allowed_error_codes option, but I'm uncertain how that fits best with the translation strings. Maybe we leave the mapping as a static method somewhere, and have the int values of the codes be the configurable part?

i.e.

SilverStripe\ErrorPage\ErrorPage:
  allowed_error_codes:
    - 400
    - 401
    - 403
    - 404
    - 500

with essentially the same PHP that we have at the moment but also with:

if (Config::get(ErrorPage::class, 'allowed_error_codes') !== null) {
  .. only filter the codes for those ones ...
}

@GuySartorelli
Copy link
Member

GuySartorelli commented Sep 25, 2023

With the exception that I'd prefer static::config()->get('allowed_error_codes'), that sounds like a good solution to me.

@GuySartorelli
Copy link
Member

Thanks for contributing that! What a great feature. It'll be included in Silverstripe CMS 5.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants