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

Jump to bottom

Deprecate LogoutButton #2899

Merged
merged 6 commits into from
Sep 5, 2024
Merged

Deprecate LogoutButton #2899

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b3f328
Outputs DeprecationWarning if LogoutButton is used in layout
leeagustin Jun 22, 2024
d6e1b58
Modifies test to check if LogoutButton use outputs DeprecationWarning
leeagustin Jun 22, 2024
c54ae91
Modifies test to show that DeprecationWarning is being output when Lo…
leeagustin Jun 22, 2024
87385a9
Refactors deprecated_types to deprecated_components
leeagustin Jun 28, 2024
3b0ab61
Merge branch 'dev' into deprecate-LogoutButton
T4rk1n Sep 5, 2024
6bd822e
Merge branch 'deprecation' into deprecate-LogoutButton
T4rk1n Sep 5, 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
35 changes: 23 additions & 12 deletions components/dash-core-components/tests/integration/location/test_location_logout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from dash.exceptions import PreventUpdate
from dash import Dash, Input, Output, dcc, html
import flask
import pytest
import time


def test_llgo001_location_logout(dash_dcc):
@pytest.mark.parametrize("add_initial_logout_button", [False, True])
def test_llgo001_location_logout(dash_dcc, add_initial_logout_button):
app = Dash(__name__)

@app.server.route("/_logout", methods=["POST"])
Expand All @@ -13,9 +15,14 @@ def on_logout():
rep.set_cookie("logout-cookie", "", 0)
return rep

app.layout = html.Div(
[html.H2("Logout test"), dcc.Location(id="location"), html.Div(id="content")]
)
layout_children = [
html.H2("Logout test"),
dcc.Location(id="location"),
html.Div(id="content"),
]
if add_initial_logout_button:
layout_children.append(dcc.LogoutButton())
app.layout = html.Div(layout_children)

@app.callback(Output("content", "children"), [Input("location", "pathname")])
def on_location(location_path):
Expand All @@ -33,15 +40,19 @@ def _insert_cookie(rep):

return dcc.LogoutButton(id="logout-btn", logout_url="/_logout")

dash_dcc.start_server(app)
time.sleep(1)
dash_dcc.percy_snapshot("Core Logout button")
with pytest.warns(
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add a condition if add_initial_logout_button for testing the deprecation notice otherwise it fails. Maybe better to just test the deprecation notice in a new test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. My worry is that DeprecationWarning is not being output when the LogoutButton is being used inside a callback—only when the LogoutButton is defined in app.layout.

This is the reason I was checking for the warning in the two separate scenarios. However, if we don't need to output a DeprecationWarning when the LogoutButton is used inside a callback, then I can just create a new test.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leeagustin can you please let us know if you added or are interested in adding the new test? thanks - @gvwilson

DeprecationWarning,
match="LogoutButton is deprecated, use a different component type instead",
):
dash_dcc.start_server(app)
time.sleep(1)
dash_dcc.percy_snapshot("Core Logout button")

assert dash_dcc.driver.get_cookie("logout-cookie")["value"] == "logged-in"
assert dash_dcc.driver.get_cookie("logout-cookie")["value"] == "logged-in"

dash_dcc.wait_for_element("#logout-btn").click()
dash_dcc.wait_for_text_to_equal("#content", "Logged out")
dash_dcc.wait_for_element("#logout-btn").click()
dash_dcc.wait_for_text_to_equal("#content", "Logged out")

assert not dash_dcc.driver.get_cookie("logout-cookie")
assert not dash_dcc.driver.get_cookie("logout-cookie")

assert dash_dcc.get_logs() == []
assert dash_dcc.get_logs() == []
12 changes: 12 additions & 0 deletions dash/_validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sys
from collections.abc import MutableSequence
import re
import warnings
from textwrap import dedent
from keyword import iskeyword
import flask
Expand Down Expand Up @@ -422,6 +423,15 @@ def validate_layout(layout, layout_value):
component_ids = set()

def _validate(value):
def _validate_type(comp):
deprecated_types = ["LogoutButton"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure there is no collision, we should also check the _namespace attribute. Also could add a custom deprecation message to the component.

I would refactor the deprecated_types to a collections.defaultdict(lambda: collections.defaultdict(dict)) then set:

deprecated_components["dash_core_components"]["LogoutButton"] = "''
The Logout Button is no longer used with Dash Enterprise and can be replaced with a html.Button or html.A.
eg: html.A(href=os.getenv('DASH_LOGOUT_URL'))
"""

The for the check you could have:

_type = getattr(comp, "_type",  "")
_ns = getattr(comp, "_namespace", "")
deprecation_message = deprecated_components[_ns][_type]
if deprecation_message:
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 87385a9

component_type = getattr(comp, "_type", None)
if component_type and component_type in deprecated_types:
warnings.warn(
f"{component_type} is deprecated, use a different component type instead",
DeprecationWarning,
)

def _validate_id(comp):
component_id = stringify_id(getattr(comp, "id", None))
if component_id and component_id in component_ids:
Expand All @@ -432,9 +442,11 @@ def _validate_id(comp):
)
component_ids.add(component_id)

_validate_type(value)
_validate_id(value)

for component in value._traverse(): # pylint: disable=protected-access
_validate_type(component)
_validate_id(component)

if isinstance(layout_value, (list, tuple)):
Expand Down