-
Notifications
You must be signed in to change notification settings - Fork 34
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
base: main
Are you sure you want to change the base?
Conversation
f9b0d8a
to
9addf91
Compare
8513197
to
62caaf8
Compare
It allows user to enter an URL.
62caaf8
to
5f2ef9e
Compare
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.
5f2ef9e
to
7c0e5a2
Compare
target="_blank" rel="noopener"> | ||
Send a link to docs section | ||
</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target="_blank" rel="noopener"> | |
Send a link to docs section | |
</a> | |
target="_blank" rel="noopener"> | |
Send a link to the Notify API documentation | |
</a> |
There was a problem hiding this comment.
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" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% set page_title = "Send a link to an API docs section" %} | |
{% set page_title = "Send a link to the Notify API documentation" %} |
There was a problem hiding this comment.
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.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel free to ignore
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'.") |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
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") |
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"]) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 🙌🏼
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 |
@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? |
|
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", |
There was a problem hiding this comment.
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" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% 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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Which version of the docs would you like to view?", | |
"Choose a client library", |
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
✨
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
Enter URL
Choose language version of the docs