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

Removes large pypi image in Slack link unfurling #17282

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
39 changes: 39 additions & 0 deletions tests/unit/utils/test_user_agents.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from warehouse.utils.user_agents import should_show_share_image


def test_shows_share_image_for_social_networks() -> None:
# https://developer.x.com/en/docs/x-for-websites/cards/guides/troubleshooting-cards#validate_twitterbot
assert should_show_share_image("Twitterbot/1.0") is True
# https://developers.facebook.com/docs/sharing/webmasters/web-crawlers
assert (
should_show_share_image(
"facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)"
)
is True
)
assert should_show_share_image("facebookexternalhit/1.1") is True
assert should_show_share_image("facebookcatalog/1.0") is True
# https://www.linkedin.com/robots.txt
assert should_show_share_image("LinkedInBot") is True


def test_doesnt_show_share_image_for_slackbot() -> None:
# https://api.slack.com/robots
assert (
should_show_share_image(
"Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)"
)
is False
)
2 changes: 2 additions & 0 deletions warehouse/packaging/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from warehouse.observations.models import ObservationKind
from warehouse.packaging.forms import SubmitMalwareObservationForm
from warehouse.packaging.models import Description, File, Project, Release, Role
from warehouse.utils.user_agents import should_show_share_image


class PEP740AttestationViewer:
Expand Down Expand Up @@ -288,6 +289,7 @@ def release_detail(release, request):
"license": license,
# Additional function to format the attestations
"PEP740AttestationViewer": PEP740AttestationViewer,
"show_share_image": should_show_share_image(request),
Copy link
Member

Choose a reason for hiding this comment

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

I think it probably makes more sense to pass the user agent here...

Suggested change
"show_share_image": should_show_share_image(request),
"user_agent": request.user_agent,

Copy link
Author

Choose a reason for hiding this comment

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

Thanks - I used the request that's already in the template (lmk if that's not always present). I also added some handling for Request.user_agent is None in case the user-agent isn't supplied.

}


Expand Down
4 changes: 4 additions & 0 deletions warehouse/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,13 @@
<meta property="og:url" content="{% if request.matched_route %}{{ request.current_route_url() }}{% else %}{{ request.url }}{% endif %}">
<meta property="og:site_name" content="PyPI">
<meta property="og:type" content="website">
{% if show_share_image %}
rmasters marked this conversation as resolved.
Show resolved Hide resolved
<meta property="og:image" content="{% block image %}{{ request.static_url('warehouse:static/dist/images/twitter.jpg') }}{% endblock %}">
{% endif %}
<meta property="og:title" content="{{ self.title()|default('Python Package Index') }}">
<meta property="og:description" content="{{ self.description() }}">
<meta name="twitter:card" content="summary">
<meta name="twitter:site" content="@pypi">
{% block extra_meta %}{% endblock %}

<link rel="search" type="application/opensearchdescription+xml" title="PyPI" href="{{ request.route_path('opensearch.xml') }}">
Expand Down
21 changes: 21 additions & 0 deletions warehouse/utils/user_agents.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from pyramid.request import Request


def should_show_share_image(request: Request) -> bool:
if user_agent := request.user_agent:
if user_agent.strip().startswith("Slackbot-LinkExpanding"):
return False

return True