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

Add a global variable to disable search autocomplete #2832

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

kevindew
Copy link
Member

@kevindew kevindew commented Dec 9, 2024

The new search autocomplete is due to be rolled out across GOV.UK.

Autocomplete uses a denylist to stop certain terms from serving suggestions. The process to update the denylist is very manual and developer heavy.

If might be difficult to update the denylist over the holiday period. In that situation the decision might be taken to temporarily turn off autocomplete entirely.

A global environment variable has been added to each environment rather than individual environment variables for each affect app, as the search bar exists in the header on all pages on GOV.UK. This means that it needs to be accessed by the layout_super_navigation_header 1 which is pulled in by static for use in 6 other rendering apps.

We are using an env var to disable this feature rather than enable it so that by default developers using these apps do not need to set-up extra env vars to use the app as it runs in production, with an expectation that it is unlikely that this feature would be disabled for a prolonged period.

When disableSearchAutcomplete is set to true an env var of GOVUK_DISABLE_SEARCH_AUTOCOMPLETE will be set with a value of "1", when disableSearchAutocomplete is false then no env var is set. Apps that use this env var can do:

if ENV["GOVUK_DISABLE_SEARCH_AUTOCOMPLETE"]
  # render search without autocomplete
else
  # render something with autocomplete
end

It is expected that once the autocomplete feature is settled in GOV.UK there will no longer be a need to disable it and this code can be removed, ideally this will happen in Q4 2024-2025 while the Search team is completing the deployment of this feature.

kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

See: alphagov/govuk-helm-charts#2832

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

See: alphagov/govuk-helm-charts#2832

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
leenagupte added a commit to alphagov/finder-frontend that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

This environment variable will either exist, or not. The value of it
doesn't really matter. If it exists, then search with autocomplete
should be disabled. A value of "1" is used in the tests, as that is the
default value being set in the helm charts.

See alphagov/govuk-helm-charts#2832
leenagupte added a commit to alphagov/frontend that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

This environment variable will either exist, or not. The value of it
doesn't really matter. If it exists, then search with autocomplete
should be disabled. A value of "1" is used in the tests, as that is the
default value being set in the helm charts.

See alphagov/govuk-helm-charts#2832
@kevindew kevindew force-pushed the all-global-disable-search-autocomplete-env-var branch from adea81a to 90e6dea Compare December 9, 2024 17:12
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether search with autocomplete
has been disabled. The value of this variable will be applied
globally across all applications that use site search.

See: alphagov/govuk-helm-charts#2832

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Dec 9, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether search with autocomplete
has been disabled. The value of this variable will be applied
globally across all applications that use site search.

See: alphagov/govuk-helm-charts#2832

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
leenagupte added a commit to alphagov/frontend that referenced this pull request Dec 10, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

This environment variable will either exist, or not. The value of it
doesn't really matter. If it exists, then search with autocomplete
should be disabled. A value of "1" is used in the tests, as that is the
default value being set in the helm charts.

See alphagov/govuk-helm-charts#2832
leenagupte added a commit to alphagov/frontend that referenced this pull request Dec 10, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

This environment variable will either exist, or not. The value of it
doesn't really matter. If it exists, then search with autocomplete
should be disabled. A value of "1" is used in the tests, as that is the
default value being set in the helm charts.

See alphagov/govuk-helm-charts#2832
leenagupte added a commit to alphagov/finder-frontend that referenced this pull request Dec 10, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether or not to render
search with autocomplete. The value of this variable will be applied
globally across all applications that use site search.

This environment variable will either exist, or not. The value of it
doesn't really matter. If it exists, then search with autocomplete
should be disabled. A value of "1" is used in the tests, as that is the
default value being set in the helm charts.

See alphagov/govuk-helm-charts#2832
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Dec 10, 2024
Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

It might be difficult to update the denylist over the holiday period. In
that situation the decision might be taken to temporarily turn off
autocomplete entirely.

An environment variable is used to determine whether search with autocomplete
has been disabled. The value of this variable will be applied
globally across all applications that use site search.

See: alphagov/govuk-helm-charts#2832

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
@kevindew kevindew marked this pull request as ready for review December 10, 2024 14:58
The new search autocomplete is due to be rolled out across GOV.UK.

Autocomplete uses a denylist to stop certain terms from serving
suggestions. The process to update the denylist is very manual and
developer heavy.

If might be difficult to update the denylist over the holiday period.
In that situation the decision might be taken to temporarily turn off
autocomplete entirely.

A global environment variable has been added to each environment rather
than individual environment variables for each affect app, as the search
bar exists in the header on all pages on GOV.UK. This means that it
needs to be accessed by the layout_super_navigation_header [1] which is pulled in by
static for use in 6 other rendering apps.

We are using an env var to disable this feature rather than enable it so
that by default developers using these apps do not need to set-up extra
env vars to use the app as it runs in production, with an expectation
that it is unlikely that this feature would be disabled for a prolonged
period.

We use a conditional in the helm chart as a means to define whether an
env var is created or not - with the expectation that the env var with
any value is a indication the feature should be disabled. When
`disableSearchAutocomplete` is set to true in a
`values-<environment>.yaml` file the `GOVUK_DISABLE_SEARCH_AUTOCOMPLETE` will
be set and have a value of "1". When `disableSearchAutocomplete` is false then
the conditional will be false and no env var is set. Apps that use this env var
can do:

```
if ENV["GOVUK_DISABLE_SEARCH_AUTOCOMPLETE"]
  # render search without autocomplete
else
  # render something with autocomplete
end
```

It is expected that once the autocomplete feature is settled in GOV.UK
there will no longer be a need to disable it and this code can be
removed, ideally this will happen in Q4 2024-2025 while the Search team
is completing the deployment of this feature.

We are starting with this feature disabled so we can turn it on
gradually to each environment.

[1]: https://github.com/alphagov/govuk_publishing_components/pull/4451/files

Co-authored-by: Leena Gupte <leena.gupte@digital.cabinet-office.gov.uk>
@kevindew kevindew force-pushed the all-global-disable-search-autocomplete-env-var branch from 90e6dea to 43c82b5 Compare December 10, 2024 15:00
@@ -16,6 +16,9 @@ data:
GOVUK_APP_DOMAIN_EXTERNAL: {{ .Values.publishingDomainSuffix }}
GOVUK_ASSET_ROOT: https://{{ .Values.assetsDomain }}
GOVUK_CSP_REPORT_URI: {{ .Values.cspReportURI | quote }}
{{- if .Values.disableSearchAutocomplete }}
GOVUK_DISABLE_SEARCH_AUTOCOMPLETE: "1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't actually matter as we expect applications to check just for the presence of the env var. I went for "1" rather than "true" as I didn't want to create an impression that "false" would override it - as any env var value will be an indication of disabling.

Copy link
Contributor

@nimalank7 nimalank7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kevindew kevindew merged commit e1c396d into main Dec 10, 2024
4 checks passed
@kevindew kevindew deleted the all-global-disable-search-autocomplete-env-var branch December 10, 2024 15:09
Copy link
Contributor

@theseanything theseanything left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work - not sure it's the clearest way to state the logic.

Having an env var:
GOVUK_ENABLE_SEARCH_AUTOCOMPLETE
which can be set to true or false (the default being true).

Then in env-configmap.yaml:

GOVUK_ENABLE_SEARCH_AUTOCOMPLETE:  {{ .Values.govukEnableSearchAutocomplete | default "true" }}

If needed to be disabled, you just add the following into the environment's value file:

govukEnableSearchAutocomplete: "false"

Avoids double negatives and having to reason about what happens if the env var is missing entirely.

Realise this is a minor change that is likely to be removed - happy for it to go in as is. But considerations for future work that require setting a global env var.

@kevindew
Copy link
Member Author

Thanks @theseanything we went down this route to avoid each of the numerous apps that use this needing to set-up the env var for local dev. Where the only way the suggestion you have would work for those is if they did a check of if ENV["GOVUK_ENABLE_SEARCH_AUTOCOMPLETE"] == "false" and that also feels strange to have a variable named "enable" but could only be used to disable.

I know not the most clean, but I don't think there is a purely clean route with boolean env vars.

kevindew added a commit to alphagov/govuk-e2e-tests that referenced this pull request Dec 12, 2024
With the introduction of autocomplete for GOV.UK Site Search this test
has started failing. The reason this is failing is that the auocomplete
component makes use of an input with a role of "combobox" and thus
locating an element with a role of "searchbox" fails.

Rather than changing the role to "combobox" I have taken the approach of
finding by label so that this can work with both input types. The reason
for is that autcomplete is new and we have set-up a mechanism so it can
be turned off quickly [1], should we do that it would be frustrating if
the e2e tests started failing.

[1]: alphagov/govuk-helm-charts#2832
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants