Skip to content

WEBBDC-2835: Add read-only phones/mails/urls datagrids to the contact section - #91

Open
boulch wants to merge 1 commit into
mainfrom
WEBBDC-2835
Open

WEBBDC-2835: Add read-only phones/mails/urls datagrids to the contact section#91
boulch wants to merge 1 commit into
mainfrom
WEBBDC-2835

Conversation

@boulch

@boulch boulch commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Contact sections can now load phone numbers, email addresses, and URLs from related contacts via a dedicated action.
    • Added read-only phone/mail/URL grids that let you choose which fields are shown per row (checkbox-controlled visible columns); rows can hide when nothing is selected.
    • Contact rendering now uses translated types, frozen-label style values, conditional links/icons, and shows VAT only when available.
  • Bug Fixes

    • Prevented empty contact-information blocks and orphaned markup.
    • Preserved column visibility selections across grid reloads and saved state.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The contact section adds read-only phone, mail, and URL datagrids loaded from related contacts. Users can select visible columns per row, with preferences persisted and rows omitted when all columns are hidden. Templates now render contact information according to those selections.

Changes

Contact information grids

Layer / File(s) Summary
Display-row schemas and frozen widgets
src/imio/smartweb/core/contents/sections/contact/content.py, src/imio/smartweb/core/vocabularies.py, src/imio/smartweb/core/vocabularies.zcml, src/imio/smartweb/core/widgets/frozen_label.py, src/imio/smartweb/core/tests/test_frozen_label.py, src/imio/smartweb/core/tests/test_vocabularies.py
Adds read-only phone, mail, and URL row schemas, selectable display-column vocabularies, and a frozen-label widget that preserves hidden form values.
Remote row construction and preferences
src/imio/smartweb/core/contents/sections/contact/utils.py, src/imio/smartweb/core/tests/resources/json_contact_informations_raw_mock.json, src/imio/smartweb/core/tests/test_section_contact.py
Fetches related contacts, builds keyed display rows, translates type labels, and applies stored visible-column preferences.
Form grid loading and persistence
src/imio/smartweb/core/contents/sections/contact/forms.py, src/imio/smartweb/core/tests/test_section_contact_forms.py
Adds shared add/edit form handling for loading related contact data, preserving checkbox selections, handling directory responses, and saving rebuilt grids.
Column-aware contact rendering
src/imio/smartweb/core/contents/sections/contact/macros.pt, src/imio/smartweb/core/tests/test_section_contact.py
Renders phones, mails, URLs, and VAT according to selected columns, including icon-only URL cases and omission of empty information blocks.
Release note
CHANGES.rst
Documents the unreleased 1.4.56 contact display-grid changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Editor
  participant ContactForm
  participant Directory
  participant ContactSection
  participant ContactMacro
  Editor->>ContactForm: select related contacts and load information
  ContactForm->>Directory: request contact information
  Directory-->>ContactForm: return phone, mail, and URL rows
  ContactForm->>ContactSection: store rows and visible_columns
  ContactSection->>ContactMacro: provide displayed_rows
  ContactMacro-->>Editor: render selected contact information
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.23% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding read-only phones, mails, and URLs datagrids to the contact section.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch WEBBDC-2835

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/imio/smartweb/core/contents/sections/contact/macros.pt (1)

110-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated social-icon chain.

Lines 110-129 and 136-155 are the same six-branch type→icon mapping; adding or renaming a network now means editing both copies, and they will drift silently (the exhaustive tests only cover facebook). Extract it once as a metal:define-macro and metal:use-macro it from both the linked and icon-only paths — or, better, drive it from a {type: icon-class} mapping exposed by ContactProperties and render a single <i tal:attributes="class ...">.

♻️ Sketch: one macro, two call sites
<metal:block define-macro="social_icon">
  <i tal:define="klass python:{'facebook': 'bi-facebook',
                               'twitter': 'bi-twitter',
                               'instagram': 'bi-instagram',
                               'youtube': 'bi-youtube',
                               'pinterest': 'bi-pinterest',
                               'linkedin': 'bi-linkedin'}.get(url.get('type'))"
     tal:condition="klass"
     tal:attributes="class string:bi ${klass}"></i>
</metal:block>

then <tal:icon tal:condition="python:'type' in columns"><metal:use use-macro="template/macros/social_icon" /></tal:icon> in both places.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/macros.pt` around lines 110
- 155, Consolidate the duplicated social type-to-icon mapping used by the linked
icon block and icon-only block into one reusable `social_icon` macro, or a
single mapping supplied by `ContactProperties`. Update both `tal:icon` and
`tal:icon_only` call sites to reuse it, preserving the existing six network
mappings and rendering no icon for unsupported types.
src/imio/smartweb/core/contents/sections/contact/utils.py (1)

52-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Two sources of truth for the column tuples.

CONTACT_ROW_COLUMNS mirrors the *DisplayColumnsVocabularyFactory.columns tuples in src/imio/smartweb/core/vocabularies.py (lines 929-954), enforced only by this comment. Drift is silent: a token added to a vocabulary but not here gets filtered out by set(columns) & all_columns in displayed_rows, and the existing vocabulary tests assert literal token lists rather than agreement between the two. Cheapest guard is a test asserting equality both ways (importing utils from vocabularies risks an import cycle).

♻️ Suggested test guard (e.g. in tests/test_vocabularies.py)
def test_display_columns_match_the_row_schema(self):
    from imio.smartweb.core.contents.sections.contact.utils import (
        CONTACT_ROW_COLUMNS,
    )

    for kind, name in (
        ("phones", "PhoneDisplayColumns"),
        ("mails", "MailDisplayColumns"),
        ("urls", "UrlDisplayColumns"),
    ):
        factory = getUtility(
            IVocabularyFactory, "imio.smartweb.vocabulary.{}".format(name)
        )
        self.assertEqual(
            [term.token for term in factory()],
            list(CONTACT_ROW_COLUMNS[kind]),
            kind,
        )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/utils.py` around lines 52 -
58, The vocabulary tests need a regression guard ensuring display-column
vocabularies stay synchronized with CONTACT_ROW_COLUMNS. Add a test in the
existing vocabulary test suite that imports CONTACT_ROW_COLUMNS locally,
retrieves the PhoneDisplayColumns, MailDisplayColumns, and UrlDisplayColumns
factories, and asserts each factory’s token list equals the corresponding
row-schema tuple in both order and contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/imio/smartweb/core/contents/sections/contact/utils.py`:
- Around line 113-132: Update get_remote_contacts to percent-encode every
submitted UID when constructing the directory `@search` query, using
urllib.parse.urlencode rather than interpolating values into the URL. Preserve
UID ordering and the existing language filter, and adjust URL-based tests to
expect the encoded query values.

---

Nitpick comments:
In `@src/imio/smartweb/core/contents/sections/contact/macros.pt`:
- Around line 110-155: Consolidate the duplicated social type-to-icon mapping
used by the linked icon block and icon-only block into one reusable
`social_icon` macro, or a single mapping supplied by `ContactProperties`. Update
both `tal:icon` and `tal:icon_only` call sites to reuse it, preserving the
existing six network mappings and rendering no icon for unsupported types.

In `@src/imio/smartweb/core/contents/sections/contact/utils.py`:
- Around line 52-58: The vocabulary tests need a regression guard ensuring
display-column vocabularies stay synchronized with CONTACT_ROW_COLUMNS. Add a
test in the existing vocabulary test suite that imports CONTACT_ROW_COLUMNS
locally, retrieves the PhoneDisplayColumns, MailDisplayColumns, and
UrlDisplayColumns factories, and asserts each factory’s token list equals the
corresponding row-schema tuple in both order and contents.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9045d81e-3d1d-4d51-9b13-c33fac7cdf44

📥 Commits

Reviewing files that changed from the base of the PR and between 24d0727 and bceb181.

📒 Files selected for processing (13)
  • CHANGES.rst
  • src/imio/smartweb/core/contents/sections/contact/content.py
  • src/imio/smartweb/core/contents/sections/contact/forms.py
  • src/imio/smartweb/core/contents/sections/contact/macros.pt
  • src/imio/smartweb/core/contents/sections/contact/utils.py
  • src/imio/smartweb/core/tests/resources/json_contact_informations_raw_mock.json
  • src/imio/smartweb/core/tests/test_frozen_label.py
  • src/imio/smartweb/core/tests/test_section_contact.py
  • src/imio/smartweb/core/tests/test_section_contact_forms.py
  • src/imio/smartweb/core/tests/test_vocabularies.py
  • src/imio/smartweb/core/vocabularies.py
  • src/imio/smartweb/core/vocabularies.zcml
  • src/imio/smartweb/core/widgets/frozen_label.py

Comment on lines +113 to +132
def get_remote_contacts(uids):
"""Live directory payload for `uids`, in that order.

Deliberately uncached: this is only called from the "load contacts
informations" button, where the editor is asking for fresh data.
"""
if not uids:
return []
url = "{}/@search?UID={}&fullobjects=1".format(DIRECTORY_URL, "&UID=".join(uids))
current_lang = api.portal.get_current_language()[:2]
if current_lang != "fr":
url = f"{url}&translated_in_{current_lang}=1"
json_data = get_json(url)
if not json_data:
return []
index_map = {uid: index for index, uid in enumerate(uids)}
items = [
item for item in json_data.get("items") or [] if item.get("UID") in index_map
]
return sorted(items, key=lambda item: index_map[item["UID"]])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Unencoded request-supplied UIDs are interpolated into the directory query.

uids reaches this function straight from the submitted form (_submitted_contact_uids in src/imio/smartweb/core/contents/sections/contact/forms.py, line 101) with no validation against the related_contacts vocabulary and no quoting. A crafted value can inject extra query parameters into the directory @search (e.g. uid&portal_type=...&review_state=private), altering what the load button pulls back. The host/path are fixed by DIRECTORY_URL, so this is parameter injection rather than SSRF, but it is cheap to close.

🔒️ Proposed fix: percent-encode the query
-    url = "{}/@search?UID={}&fullobjects=1".format(DIRECTORY_URL, "&UID=".join(uids))
+    query = [("UID", uid) for uid in uids]
+    query.append(("fullobjects", "1"))
     current_lang = api.portal.get_current_language()[:2]
     if current_lang != "fr":
-        url = f"{url}&translated_in_{current_lang}=1"
+        query.append(("translated_in_{}".format(current_lang), "1"))
+    url = "{}/@search?{}".format(DIRECTORY_URL, urlencode(query))

with from urllib.parse import urlencode at the top. Note the existing tests build their mocked URLs by hand, so they will need the same encoding (unchanged for plain hex UIDs).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def get_remote_contacts(uids):
"""Live directory payload for `uids`, in that order.
Deliberately uncached: this is only called from the "load contacts
informations" button, where the editor is asking for fresh data.
"""
if not uids:
return []
url = "{}/@search?UID={}&fullobjects=1".format(DIRECTORY_URL, "&UID=".join(uids))
current_lang = api.portal.get_current_language()[:2]
if current_lang != "fr":
url = f"{url}&translated_in_{current_lang}=1"
json_data = get_json(url)
if not json_data:
return []
index_map = {uid: index for index, uid in enumerate(uids)}
items = [
item for item in json_data.get("items") or [] if item.get("UID") in index_map
]
return sorted(items, key=lambda item: index_map[item["UID"]])
def get_remote_contacts(uids):
"""Live directory payload for `uids`, in that order.
Deliberately uncached: this is only called from the "load contacts
informations" button, where the editor is asking for fresh data.
"""
if not uids:
return []
query = [("UID", uid) for uid in uids]
query.append(("fullobjects", "1"))
current_lang = api.portal.get_current_language()[:2]
if current_lang != "fr":
query.append(("translated_in_{}".format(current_lang), "1"))
url = "{}/@search?{}".format(DIRECTORY_URL, urlencode(query))
json_data = get_json(url)
if not json_data:
return []
index_map = {uid: index for index, uid in enumerate(uids)}
items = [
item for item in json_data.get("items") or [] if item.get("UID") in index_map
]
return sorted(items, key=lambda item: index_map[item["UID"]])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/utils.py` around lines 113 -
132, Update get_remote_contacts to percent-encode every submitted UID when
constructing the directory `@search` query, using urllib.parse.urlencode rather
than interpolating values into the URL. Preserve UID ordering and the existing
language filter, and adjust URL-based tests to expect the encoded query values.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/imio/smartweb/core/contents/sections/contact/macros.pt (1)

110-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated type→icon mapping (6-way tal:condition chain repeated twice).

The facebook/twitter/instagram/youtube/pinterest/linkedin icon-selection block is fully duplicated between the "wrapped in <a>" case (110-129) and the "icon_only" case (136-155). Consider extracting a single icon-class lookup (e.g., a small dict/method on the view returning the bootstrap icon class for a given type) so the template only needs one conditional icon element, reused in both branches.

♻️ Illustrative refactor sketch
-                    <tal:icon tal:condition="python:'type' in columns">
-                      <tal:condition tal:condition="python:url.get('type') == 'facebook'">
-                          <i class="bi bi-facebook"></i>
-                      </tal:condition>
-                      <tal:condition tal:condition="python:url.get('type') == 'twitter'">
-                          <i class="bi bi-twitter"></i>
-                      </tal:condition>
-                      ... (4 more conditions) ...
-                    </tal:icon>
+                    <i tal:condition="python:'type' in columns and contact.get_social_icon_class(url.get('type'))"
+                       tal:attributes="class python:'bi {}'.format(contact.get_social_icon_class(url.get('type')))"></i>

Then reuse the same <i tal:condition=... > snippet for the icon_only branch, eliminating the second full copy.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/macros.pt` around lines 110
- 155, Remove the duplicated six-way icon-selection chain from the template by
introducing one reusable type-to-Bootstrap-icon lookup and a shared
icon-rendering element. Update both the linked branch and the icon_only branch
to use this lookup for facebook, twitter, instagram, youtube, pinterest, and
linkedin, preserving the existing conditions and output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/imio/smartweb/core/contents/sections/contact/macros.pt`:
- Around line 101-109: Update the social link rendered in the repeated
url_entries block to include rel="noopener noreferrer" alongside target="_blank"
on the anchor element, preserving its existing href, title, and condition
behavior.
- Around line 76-99: Update the phone and mail link attributes and contents in
the repeated phone/mail entries to use an empty-string fallback when number or
mail_address is missing, matching the existing label handling. Ensure missing
values never render as “None” or produce tel:None/mailto:None while preserving
normal values.

---

Nitpick comments:
In `@src/imio/smartweb/core/contents/sections/contact/macros.pt`:
- Around line 110-155: Remove the duplicated six-way icon-selection chain from
the template by introducing one reusable type-to-Bootstrap-icon lookup and a
shared icon-rendering element. Update both the linked branch and the icon_only
branch to use this lookup for facebook, twitter, instagram, youtube, pinterest,
and linkedin, preserving the existing conditions and output.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17743569-9bcc-4763-ab2b-502ed49da301

📥 Commits

Reviewing files that changed from the base of the PR and between bceb181 and 013b23d.

📒 Files selected for processing (13)
  • CHANGES.rst
  • src/imio/smartweb/core/contents/sections/contact/content.py
  • src/imio/smartweb/core/contents/sections/contact/forms.py
  • src/imio/smartweb/core/contents/sections/contact/macros.pt
  • src/imio/smartweb/core/contents/sections/contact/utils.py
  • src/imio/smartweb/core/tests/resources/json_contact_informations_raw_mock.json
  • src/imio/smartweb/core/tests/test_frozen_label.py
  • src/imio/smartweb/core/tests/test_section_contact.py
  • src/imio/smartweb/core/tests/test_section_contact_forms.py
  • src/imio/smartweb/core/tests/test_vocabularies.py
  • src/imio/smartweb/core/vocabularies.py
  • src/imio/smartweb/core/vocabularies.zcml
  • src/imio/smartweb/core/widgets/frozen_label.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/imio/smartweb/core/tests/test_frozen_label.py
  • CHANGES.rst

Comment on lines +76 to +99
<tal:phone tal:repeat="entry phone_entries">
<li tal:define="phone entry/data; columns entry/columns"
tal:attributes="class python:phone.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:phone.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('phones', phone.get('type'))"></span>
<a tal:condition="python:'number' in columns"
tal:attributes="href python:'tel:{}'.format(phone.get('number'))"
tal:content="python:'{}'.format(phone.get('number'))" />
</li>
</tal:phone>
<tal:mail tal:repeat="entry mail_entries">
<li tal:define="mail entry/data; columns entry/columns"
tal:attributes="class python:mail.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:mail.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('mails', mail.get('type'))"></span>
<a tal:condition="python:'mail_address' in columns"
tal:attributes="href python:'mailto:{}'.format(mail.get('mail_address'))"
tal:content="python:'{}'.format(mail.get('mail_address'))" />
</li>
</tal:mail>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard number/mail_address against missing values to avoid literal tel:None / mailto:None.

'tel:{}'.format(phone.get('number')) and '{}'.format(phone.get('number')) render the string "None" when the number key is missing/empty for a row even though the number column is selected — producing a broken tel:None link with visible text "None". Same issue for mail_address/mailto:. Note the label span two lines above already guards with or ''; number/mail_address lack the equivalent fallback.

🐛 Proposed fix
-                <a tal:condition="python:'number' in columns"
+                <a tal:condition="python:'number' in columns and phone.get('number')"
                    tal:attributes="href python:'tel:{}'.format(phone.get('number'))"
-                   tal:content="python:'{}'.format(phone.get('number'))" />
+                   tal:content="python:phone.get('number')" />
-                <a tal:condition="python:'mail_address' in columns"
+                <a tal:condition="python:'mail_address' in columns and mail.get('mail_address')"
                    tal:attributes="href python:'mailto:{}'.format(mail.get('mail_address'))"
-                   tal:content="python:'{}'.format(mail.get('mail_address'))" />
+                   tal:content="python:mail.get('mail_address')" />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<tal:phone tal:repeat="entry phone_entries">
<li tal:define="phone entry/data; columns entry/columns"
tal:attributes="class python:phone.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:phone.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('phones', phone.get('type'))"></span>
<a tal:condition="python:'number' in columns"
tal:attributes="href python:'tel:{}'.format(phone.get('number'))"
tal:content="python:'{}'.format(phone.get('number'))" />
</li>
</tal:phone>
<tal:mail tal:repeat="entry mail_entries">
<li tal:define="mail entry/data; columns entry/columns"
tal:attributes="class python:mail.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:mail.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('mails', mail.get('type'))"></span>
<a tal:condition="python:'mail_address' in columns"
tal:attributes="href python:'mailto:{}'.format(mail.get('mail_address'))"
tal:content="python:'{}'.format(mail.get('mail_address'))" />
</li>
</tal:mail>
<tal:phone tal:repeat="entry phone_entries">
<li tal:define="phone entry/data; columns entry/columns"
tal:attributes="class python:phone.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:phone.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('phones', phone.get('type'))"></span>
<a tal:condition="python:'number' in columns and phone.get('number')"
tal:attributes="href python:'tel:{}'.format(phone.get('number'))"
tal:content="python:phone.get('number')" />
</li>
</tal:phone>
<tal:mail tal:repeat="entry mail_entries">
<li tal:define="mail entry/data; columns entry/columns"
tal:attributes="class python:mail.get('type') or ''">
<span tal:condition="python:'label' in columns"
tal:replace="python:mail.get('label') or ''"></span>
<span tal:condition="python:'type' in columns"
tal:replace="python:contact.translated_type('mails', mail.get('type'))"></span>
<a tal:condition="python:'mail_address' in columns and mail.get('mail_address')"
tal:attributes="href python:'mailto:{}'.format(mail.get('mail_address'))"
tal:content="python:mail.get('mail_address')" />
</li>
</tal:mail>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/macros.pt` around lines 76 -
99, Update the phone and mail link attributes and contents in the repeated
phone/mail entries to use an empty-string fallback when number or mail_address
is missing, matching the existing label handling. Ensure missing values never
render as “None” or produce tel:None/mailto:None while preserving normal values.

Comment on lines 101 to +109
<ul class="contact_informations_social"
tal:define="urls python:contact.get_urls"
tal:condition="urls">
<li class="urls"
tal:repeat="url urls"
tal:attributes="class python:url.get('type') or ''">
<a tal:attributes="href python:url.get('url');
title python:contact.get_translated_url_type(url.get('type'))"
target="_blank">
tal:condition="url_entries">
<tal:url tal:repeat="entry url_entries">
<li tal:define="url entry/data; columns entry/columns"
tal:attributes="class python:url.get('type') or ''">
<a tal:condition="python:'url' in columns"
tal:attributes="href python:url.get('url');
title python:contact.get_translated_url_type(url.get('type')) if ('type' in columns and url.get('type')) else nothing"
target="_blank">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Add rel="noopener noreferrer" to the target="_blank" social link.

Opening external URLs with target="_blank" and no rel="noopener noreferrer" exposes the page to reverse-tabnabbing (the opened page can access window.opener).

🛡️ Proposed fix
                 <a tal:condition="python:'url' in columns"
                    tal:attributes="href python:url.get('url');
                                    title python:contact.get_translated_url_type(url.get('type')) if ('type' in columns and url.get('type')) else nothing"
+                   rel="noopener noreferrer"
                    target="_blank">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ul class="contact_informations_social"
tal:define="urls python:contact.get_urls"
tal:condition="urls">
<li class="urls"
tal:repeat="url urls"
tal:attributes="class python:url.get('type') or ''">
<a tal:attributes="href python:url.get('url');
title python:contact.get_translated_url_type(url.get('type'))"
target="_blank">
tal:condition="url_entries">
<tal:url tal:repeat="entry url_entries">
<li tal:define="url entry/data; columns entry/columns"
tal:attributes="class python:url.get('type') or ''">
<a tal:condition="python:'url' in columns"
tal:attributes="href python:url.get('url');
title python:contact.get_translated_url_type(url.get('type')) if ('type' in columns and url.get('type')) else nothing"
target="_blank">
<ul class="contact_informations_social"
tal:condition="url_entries">
<tal:url tal:repeat="entry url_entries">
<li tal:define="url entry/data; columns entry/columns"
tal:attributes="class python:url.get('type') or ''">
<a tal:condition="python:'url' in columns"
tal:attributes="href python:url.get('url');
title python:contact.get_translated_url_type(url.get('type')) if ('type' in columns and url.get('type')) else nothing"
rel="noopener noreferrer"
target="_blank">
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/smartweb/core/contents/sections/contact/macros.pt` around lines 101
- 109, Update the social link rendered in the repeated url_entries block to
include rel="noopener noreferrer" alongside target="_blank" on the anchor
element, preserving its existing href, title, and condition behavior.

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.

1 participant