Admin > Settings > General page should have only one path

Summary

There are currently 2 paths for Admin Area > Settings > General page: /admin/application_settings and /admin/application_settings/general. And both paths are actually used: if you click "Settings" on the sidebar you'll go to /admin/application_settings, if you click "General" you'll go to /admin/application_settings/general. This could possibly lead to bugs, and in fact it does: #43657 (closed), #43658 (closed).

Some of GitLab features try to identify a page by its path (nav_link helper, Webpack code splitting config). Since GitLab rely on that, it'd be simpler if each page had only one path. That way you don't need to check config/routes.rb or rake routes each time you use nav_link or similar feature. You won't forget to add all aliases. You can write less code: nav_link(path: '/foo') instead of nav_link(path: %w(/foo /bar /other/foo/aliases)). Aliases, if needed, could be implemented with redirects.

So I suggest to choose a single path for the Admin Area > Settings > General page (let's say /admin/application_settings/general) and stick to it. For the other path (/admin/application_settings) I have several proposals, which I'll describe below.

Possible solutions

1. Remove the other path

/admin/application_settings can be removed. It's a bit odd that there will be nested resources /admin/application_settings/{general,integrations,ci_cd,etc.} and no higher level resource /admin/application_settings. But such solution is consistent with how it is in project settings: there are /project-name/-/settings/{repository,ci_cd,audit_events} and yet no /project-name/-/settings. So it might be a compromise solution.

Pros:

  • we're explicit in that there is no /admin/application_settings
  • consistent with /project-name/-/settings

Cons:

  • slightly more typing in the address bar
  • need to change paths in docs (it's preferable to change them anyway)
  • someone curious may type that path and get 404

2. Redirect from the other path to the main path

Requests to /admin/application_settings can be redirected to /admin/applications_settings/general. This is a way to prevent bugs like the ones mentioned above without changing behavior for users (if there are users that navigate by typing a path). It also might be a compromise solution.

Pros:

  • slightly less typing in the address bar
  • no need to change paths in docs (it's preferable to change them though)
  • probably better than 404

Cons:

  • there are no examples at GitLab of redirect from a nested resource to a parent
  • it might be surprising for someone expecting to find a parent resource at this path

3. Change the page accessible at the other path

Another page can be located at /admin/application_settings. As a parent resource, it might be a list or table of the nested resources: links to general, integration, CI/CD settings. Probably not worth it, unrealistic.

Pros:

  • definitely better than 404
  • meets expectations: it's a parent resource at a higher-level path

Cons:

  • inconsistent with the current design, there are no examples of such pages
  • not sure if it will have value for users
  • requires more work than other solutions (design, frontend)

4. Rethink URI structure

The whole URI structure under /admin/application_settings may be changed so that there is no nested resources and therefore no such problem as a missing higher level resource. This will require a lot of work too, but otherwise it's probably the perfect solution.

Improvements

Any of these solutions will make Admin Area > Settings > General page identifiable by exactly one path. This will allow to write less code. This will prevent bugs like the ones mentioned above.

Risks

For solutions 1, 2 and 3 all admin_application_settings_path will need to be changed to general_admin_application_settings_path throughout the code. And all /admin/application_settings in will need to be changed to /admin/application_settings/general throughout the docs. There is a risk to miss something, but tests + grep should help to minimize the risk.

Solution 4 requires more changes like that, so it's more risky in this sense, more chances to miss something.

Involved components

The following files will need to be changed (for solution 4 probably some more):

app/controllers/admin/application_settings_controller.rb
app/views/admin/dashboard/index.html.haml
app/views/layouts/nav/sidebar/_admin.html.haml
config/routes/admin.rb
doc/administration/geo/replication/configuration.md
doc/administration/raketasks/project_import_export.md
doc/api/settings.md
doc/development/documentation/styleguide.md
doc/security/password_length_limits.md
doc/security/ssh_keys_restrictions.md
doc/security/two_factor_authentication.md
doc/security/user_email_confirmation.md
doc/user/project/settings/import_export.md
ee/spec/controllers/admin/application_settings_controller_spec.rb
spec/controllers/admin/application_settings_controller_spec.rb
spec/features/admin/admin_disables_git_access_protocol_spec.rb
spec/features/admin/admin_settings_spec.rb

Intended side effects

This issues will be automatically fixed: #43658 (closed), #43657 (closed)

Edited by A. I. Oleynikov