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.

Sign up for GitHub

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

Squash a few routing / UI bugs #1957

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Squash a few routing / UI bugs #1957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
842ccaf
Squash a few routing / UI bugs
whabanks Sep 25, 2024
3474d19
Squash validation bug
whabanks Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 53 additions & 16 deletions app/main/views/api_keys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@
"default_with_tick")
return redirect(url_for(back_link, service_id=service_id))

flash(["{}".format(
_l("Are you sure you want to delete this callback configuration?"))], "delete")
flash(["{}".format(
_l("Are you sure you want to delete this callback configuration?"))], "delete")

form = ServiceDeliveryStatusCallbackForm(
url=delivery_status_callback.get(
Expand Down Expand Up @@ -250,26 +250,34 @@
callback_api_id=delivery_status_callback.get("id"),
)

# If the user is just testing their URL, don't send them back to the API Integration page
if request.form.get("button_pressed") == "test_response_time":
flash(
_l("The service {} responded in {} seconds.").format(
url_hostname,
response_time,
),
"default_with_tick",
)
return redirect(url_for("main.delivery_status_callback", service_id=service_id))

# If the user is just testing their URL, don't send them back to the API Integration page
if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

in this case is the callback saved or not? the flash message doesn't indicate that, whereas if it is <1s, it does say the callback is saved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm good catch on this case. Right now, if the url or bearer token have changed it will call the update method, then check if the test button was clicked and display the response time.

I think what we want is to check if the test button was clicked first, and put the url/bearer token check + update call in the else. It would be confusing to perform a save and tell them that after they click the test button.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok we dont need to change this for now, I guess it is saved? but maybe we need stronger language? your callback should respond <1s for us to count it performant or something like that

flash(
_l("We’ve saved your callback configuration. {} responded in {} seconds.").format(
_l("The service {} took longer than 1 second to respond.").format(
url_hostname,
),
"error",
)
return redirect(url_for("main.delivery_status_callback", service_id=service_id))
else:
flash(
_l("The service {} responded in {} seconds.").format(
url_hostname,
response_time,
),
"default_with_tick",
)
return redirect(url_for("main.delivery_status_callback", service_id=service_id))

return redirect(url_for(back_link, service_id=service_id))
flash(
_l("We’ve saved your callback configuration. {} responded in {} seconds.").format(
url_hostname,
response_time,
),
"default_with_tick",
)
Dismissed Show dismissed Hide dismissed

return redirect(url_for(back_link, service_id=service_id))
# Create a new callback
elif form.url.data:
service_api_client.create_service_callback_api(
Expand Down Expand Up @@ -312,6 +320,16 @@
)
return redirect(url_for("main.delivery_status_callback", service_id=service_id))

flash(
_l("We’ve saved your callback configuration. {} responded in {} seconds.").format(
url_hostname,
response_time,
),
"default_with_tick",
)
Dismissed Show dismissed Hide dismissed

return redirect(url_for(back_link, service_id=service_id))

return render_template(
"views/api/callbacks/delivery-status-callback.html",
has_callback_config=delivery_status_callback is not None,
Expand Down Expand Up @@ -362,7 +380,15 @@
)

# If the user is just testing their URL, don't send them back to the API Integration page
if request.form.get("button_pressed") == "test_response_time":
if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1:
flash(
_l("The service {} took longer than 1 second to respond.").format(
url_hostname,
),
"error",
)
return redirect(url_for("main.received_text_messages_callback", service_id=service_id))
else:
flash(
_l("The service {} responded in {} seconds.").format(
url_hostname,
Expand All @@ -379,6 +405,7 @@
),
"default_with_tick",
)
return redirect(url_for(back_link, service_id=service_id))

elif received_text_messages_callback and not form.url.data:
service_api_client.delete_service_inbound_api(
Expand Down Expand Up @@ -420,6 +447,16 @@
)
return redirect(url_for("main.received_text_messages_callback", service_id=service_id))

flash(
_l("We’ve saved your callback configuration. {} responded in {} seconds.").format(
url_hostname,
response_time,
),
"default_with_tick",
)
Dismissed Show dismissed Hide dismissed

return redirect(url_for(back_link, service_id=service_id))

return render_template(
"views/api/callbacks/received-text-messages-callback.html",
has_callback_config=received_text_messages_callback is not None,
Expand Down
2 changes: 1 addition & 1 deletion app/templates/components/page-footer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
button1_value="b1",
button2_text=None,
button2_value=None,
delete_link=False,
delete_link=None,
delete_link_text=_("Delete")) %}
<div class="js-stick-at-bottom-when-scrolling">
<div class="page-footer">
Expand Down
4 changes: 2 additions & 2 deletions app/templates/views/api/callbacks/delivery-status-callback.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ <h2 class="heading-small">
{% set test_response_txt = _('Test response time') if has_callback_config else None %}
{% set test_response_value = _('test_response_time') if has_callback_config else None %}
{% set display_footer = is_deleting if is_deleting else False %}
{% set delete_link = url_for('.delete_delivery_status_callback', service_id=current_service.id) if has_callback_config else None%}
{% if not display_footer %}
{{ sticky_page_footer_two_submit_buttons_and_delete_link(
button1_text=_('Save'),
button1_value=_('save'),
button2_text=test_response_txt,
button2_value=test_response_value,
delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id),
delete_link_text=_('Delete')
delete_link=delete_link,
) }}
{% endif %}
{% endcall %}
Expand Down
5 changes: 3 additions & 2 deletions app/templates/views/api/callbacks/received-text-messages-callback.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ <h2 class="heading-small">
{% set test_response_txt = _('Test response time') if has_callback_config else None %}
{% set test_response_value = _('test_response_time') if has_callback_config else None %}
{% set display_footer = is_deleting if is_deleting else False %}
{% set delete_link = url_for('.delete_received_text_messages_callback', service_id=current_service.id) if has_callback_config else None%}
{% set delete_link_text = _('Delete') if has_callback_config else None %}
{% if not display_footer %}
{{ sticky_page_footer_two_submit_buttons_and_delete_link(
button1_text=_('Save'),
button1_value=_('save'),
button2_text=test_response_txt,
button2_value=test_response_value,
delete_link=url_for('.delete_received_text_messages_callback', service_id=current_service.id),
delete_link_text=_('Delete')
delete_link=delete_link,
) }}
{% endif %}
{% endcall %}
Expand Down
Loading