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

Let user choose which version of Notify docs they want to view #5235

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

CrystalPea
Copy link
Contributor

@CrystalPea CrystalPea commented Oct 1, 2024

Let user choose which version of Notify docs they want to view - when sending them a link to a particular section in the docs.

What work has been done here

  • a page added where platform admin can submit a link to a section within Notify docs
  • when that that is submitted, a page is produced that can be sent to the service user
  • The service user who goes to that page can choose which language version they want to see our docs for
  • Upon selecting the language and submitting their selection, they are taken to the docs of their choice, to the section that our tech support wanted to share

Why do this work

We do this as often when sending service users links to a section in the docs, we don't know which of our clients they use, if any.

Notes

I also included "Any is fine" option to choose, for users who aren't very technical, but would still like to see the docs.
It might be hard for these users to choose from a list of options they are not familiar with. Hence, I provided a "comfort" option for them. Selecting this will take them to rest API docs.

Screenshots

Link to new flow - only visible to Platform Admins

Screenshot 2024-10-03 at 14 10 48

Enter URL

Screenshot 2024-10-03 at 14 41 12

Choose language version of the docs

Screenshot 2024-10-03 at 14 47 44

@CrystalPea CrystalPea force-pushed the api-docs-sections-landing-page branch from f9b0d8a to 9addf91 Compare October 2, 2024 17:15
@CrystalPea CrystalPea marked this pull request as ready for review October 3, 2024 10:55
@CrystalPea CrystalPea changed the title Api docs sections landing page Let user choose which version of Notify docs they want to view Oct 3, 2024
@CrystalPea CrystalPea force-pushed the api-docs-sections-landing-page branch 2 times, most recently from 8513197 to 62caaf8 Compare October 3, 2024 13:36
@CrystalPea CrystalPea force-pushed the api-docs-sections-landing-page branch from 62caaf8 to 5f2ef9e Compare October 3, 2024 13:38
When sending them a link to a particular section in the docs.

We do this as often when sending service users links to a section
in the docs, we don't know which of our clients they use, if any.

I also included "Any is fine" option to choose, for users who
aren't very technical, but would still like to see the docs.
It might be hard for these users to choose from a list of options
they are not familiar with. Hence, I provided a "comfort" option
for them. Selecting this will take them to rest API docs.
The link is only visible to platform admins.
@CrystalPea CrystalPea force-pushed the api-docs-sections-landing-page branch from 5f2ef9e to 7c0e5a2 Compare October 3, 2024 13:47
Comment on lines +17 to +19
target="_blank" rel="noopener">
Send a link to docs section
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
target="_blank" rel="noopener">
Send a link to docs section
</a>
target="_blank" rel="noopener">
Send a link to the Notify API documentation
</a>

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is for platform admins, do we need to specify ‘Notify’?

Can we just say the API documentation instead?


{# Used by the content_template.html layout, prefixes the "navigation" accessible name #}
{% set navigation_label_prefix = 'Using Notify' %}
{% set page_title = "Send a link to an API docs section" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% set page_title = "Send a link to an API docs section" %}
{% set page_title = "Send a link to the Notify API documentation" %}

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is for platform admins, do we need to specify ‘Notify’?

Can we just say the API documentation instead?

if parsed_url.hostname == "docs.notifications.service.gov.uk" and parsed_url.fragment:
return parsed_url
else:
raise ValidationError("Must be a valid https URL, pointing to a section within the GOV.UK Notify API docs.")
Copy link
Contributor

Choose a reason for hiding this comment

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

feel free to ignore

Suggested change
raise ValidationError("Must be a valid https URL, pointing to a section within the GOV.UK Notify API docs.")
raise ValidationError("The URL must point to a specific section in the GOV.UK Notify API docs, and start with 'https://docs.notifications.service.gov.uk'.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice one, I will add this to code (and tests)

Copy link
Contributor

@karlchillmaid karlchillmaid Oct 16, 2024

Choose a reason for hiding this comment

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

We can probably be a little more consistent with our existing validation errors.

Suggested change
raise ValidationError("Must be a valid https URL, pointing to a section within the GOV.UK Notify API docs.")
raise ValidationError("Enter a URL in the correct format, beginning https://docs.notifications.service.gov.uk")
Suggested change
raise ValidationError("Must be a valid https URL, pointing to a section within the GOV.UK Notify API docs.")
raise ValidationError("Enter a URL beginning https://docs.notifications.service.gov.uk")

@@ -159,6 +159,39 @@ def guidance_api_documentation():
)


@main.route("/using-notify/api-documentation/section", methods=["GET", "POST"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this either be platform admin only or have the @hide_from_search_engines decorator? There's nothing sensitive on the page, but it might be confusing to people if they come across it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea to hide it from search engines 🙌🏼

@klssmith
Copy link
Contributor

klssmith commented Oct 4, 2024

You've probably already seen this but not all the headings are consistent, so that might need to be looked at before this could be merged. No major differences, just things like #get-a-pdf-for-a-letter-notification vs #get-a-pdf-for-a-letter

@CrystalPea
Copy link
Contributor Author

CrystalPea commented Oct 4, 2024

@klssmith yeah, I thought they probably aren't - we have upcoming work on the team to get API docs more in line with one another - maybe we could start with making the headers match 🙌🏼 . What do you think @saimaghafoor ? Could be a good goal for first pass over the docs?

@quis
Copy link
Member

quis commented Oct 8, 2024

  • agree that we should fix the mismatched headings (comparison tool)
  • don’t think we need radio buttons here – the choices could just be links
  • I don’t like adding additional, platform-admin-only elements (the link) to user-facing pages because it confuses people on support about what users can/cannot see

@quis
Copy link
Member

quis commented Oct 8, 2024

As an interim step could we automatically validate that the header exists in all the docs before generating the link?

("rest-api", "Rest API"),
("rest-api", "Any is fine"),
],
thing="a language version of GOV.UK Notify's API docs",
Copy link
Contributor

Choose a reason for hiding this comment

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

Apart from the REST API documentation, we refer to these as ‘client libraries’ elsewhere.


{# Used by the content_template.html layout, prefixes the "navigation" accessible name #}
{% set navigation_label_prefix = 'Using Notify' %}
{% set page_title = "You have been sent a link to GOV.UK Notify API documentation" %}
Copy link
Contributor

@karlchillmaid karlchillmaid Oct 16, 2024

Choose a reason for hiding this comment

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

Suggested change
{% set page_title = "You have been sent a link to GOV.UK Notify API documentation" %}
{% set page_title = "You have been sent a link to the GOV.UK Notify API documentation" %}

{% block content_column_content %}

<h1 class="heading-large">{{page_title}}</h1>
<p class="govuk-body">Below, copy paste the full URL for docs section you would like to send to a service user, and click "Submit".</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<p class="govuk-body">Below, copy paste the full URL for docs section you would like to send to a service user, and click "Submit".</p>
<p class="govuk-body">Enter the full URL for a section of the API documentation and select <b class="govuk-!-font-weight-bold">Submit</b>.</p>

<p class="govuk-body">Below, copy paste the full URL for docs section you would like to send to a service user, and click "Submit".</p>

<p class="govuk-body">You will be taken to
a landing page for that section. Send link to that page to your user, and they will be able to choose which language version they want to see.</p>
Copy link
Contributor

@karlchillmaid karlchillmaid Oct 16, 2024

Choose a reason for hiding this comment

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

Suggested change
a landing page for that section. Send link to that page to your user, and they will be able to choose which language version they want to see.</p>
a landing page for that section. Send the URL of that page to the user. When they open it in their browser, they’ll be able to choose a client library.</p>

super().__init__(section_tag=section_tag)

docs_version = GovukRadiosField(
"Which version of the docs would you like to view?",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Which version of the docs would you like to view?",
"Choose a client library",

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.

6 participants