Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions news/URB-3595.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add dematerialized encoding warning on NOTICE folder
[WBoudabous]
3 changes: 3 additions & 0 deletions src/Products/urban/browser/cron/notice.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from Products.urban.contentrules.notice import NoticeImportFailedEvent
from Products.urban.contentrules.notice import NoticeImportSucceededEvent
from Products.urban.interfaces import IBaseBuildLicence
from Products.urban.interfaces import ILicenceCreatedViaNoticeWS
from Products.urban.notice.exceptions import ErrorProcessingNotificationException
from Products.urban.notice.exceptions import FailedGettingRecentNotificationsException
from Products.urban.notice.exceptions import NoImplementationFoundException
Expand All @@ -21,6 +22,7 @@
from zope.annotation.interfaces import IAnnotations
from zope.event import notify
from zope.i18n import translate
from zope.interface import alsoProvides
from zope.lifecycleevent import ObjectModifiedEvent

import logging
Expand Down Expand Up @@ -330,6 +332,7 @@ def create_licence(self):
self.licence = api.content.create(
container=self.notification.container, **self.notification.serialize()
)
alsoProvides(self.licence, ILicenceCreatedViaNoticeWS)
if IBaseBuildLicence.providedBy(self.licence):
self.licence.setUsage("not_applicable")
self.licence.setFoldermanagers(
Expand Down
11 changes: 10 additions & 1 deletion src/Products/urban/browser/warnings/conditions.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# -*- coding: utf-8 -*-

from Products.urban.interfaces import ILicenceCreatedViaNoticeWS
from Products.urban.interfaces import IUrbanWarningCondition
from plone import api
from zope.interface import implements


class WarningCondition(object):
"""
Base class for any object adapting a licence into a warning
Expand Down Expand Up @@ -43,3 +43,12 @@ def evaluate(self):
):
return True
return False


class NoticeWarning(WarningCondition):
"""
Check if license is a notice folder.
"""

def evaluate(self):
return ILicenceCreatedViaNoticeWS.providedBy(self.licence)
7 changes: 7 additions & 0 deletions src/Products/urban/browser/warnings/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,11 @@
name="urban.warnings.bound_ticket_settlement"
/>

<adapter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrong indent

for="Products.urban.interfaces.IGenericLicence"
provides="Products.urban.interfaces.IUrbanWarningCondition"
factory=".conditions.NoticeWarning"
name="urban.warnings.notice"
/>

</configure>
4 changes: 4 additions & 0 deletions src/Products/urban/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ class IUrbanEventNotice(Interface):
"""Marker interface for .UrbanEventNotice.UrbanEventNotice"""


class ILicenceCreatedViaNoticeWS(Interface):
"""Marker interface for a licence created through the Notice webservice"""


class IUrbanEventOpinionRequest(Interface):
"""Marker interface for .UrbanEventOpinionRequest.UrbanEventOpinionRequest"""

Expand Down
6 changes: 6 additions & 0 deletions src/Products/urban/locales/fr/LC_MESSAGES/urban.po
Original file line number Diff line number Diff line change
Expand Up @@ -5980,3 +5980,9 @@ msgstr "Plans Modificatifs - Notification à la commune que le RS vaut décision

msgid "PM_REFUS_TACITE_COMMUNE"
msgstr "Plans modificatifs - Notification à la commune du refus tacite"

msgstr "Nature de l'immeuble"
msgstr "Nature de l'immeuble"

msgid "urban.warnings.notice"
msgstr "Ce dossier a été encodé de manière dématérialisée"
40 changes: 40 additions & 0 deletions src/Products/urban/migration/update_290.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
from Products.urban.contentrules.notice import INoticeImportSucceededEvent
from Products.urban.contentrules.notice import INoticeResponseFailedEvent
from Products.urban.contentrules.utils import ContentRulesUtils
from Products.urban.interfaces import IGenericLicence
from Products.urban.interfaces import ILicenceCreatedViaNoticeWS
from Products.urban.interfaces import IUrbanEventNotice
from Products.urban.migration.utils import cook_javascript_resources
from Products.urban.setuphandlers import add_new_urban_licence_type
from Products.urban.utils import moveElementAfter
Expand All @@ -20,8 +23,11 @@
from plone.registry import field
from plone.registry import Record
from plone.registry.interfaces import IRegistry
from zope.annotation.interfaces import IAnnotations
from zope.component import getUtility
from zope.event import notify
from zope.i18n import translate
from zope.interface import alsoProvides

import logging

Expand Down Expand Up @@ -721,3 +727,37 @@ def setup_referenceFT_PM(context):
reindexIndexes(None, ["referenceFT_PM"])

logger.info("upgrade step done!")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing blank lines

def add_notice_warning(context):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This method should also add the interface mentioned for NoticeWarning class for all existing created licences related to notice

Comment on lines 729 to +730

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add two blank lines before add_notice_warning.

Line 730 follows line 729 with no separation. This breaks PEP8 E302 and repeats an earlier reviewer comment on line 729.

Proposed fix
     logger.info("upgrade step done!")
+
+
 def add_notice_warning(context):
🤖 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/Products/urban/migration/update_290.py` around lines 729 - 730, Add two
blank lines between the completion log statement and the add_notice_warning
function definition to satisfy PEP8 E302, without changing the surrounding
migration logic.

"""
Add notice warning on portal_urban warnings field.
"""
logger = logging.getLogger("urban: Add notice warning")
logger.info("starting upgrade steps")
portal_urban = api.portal.get_tool("portal_urban")
existing = list(portal_urban.getWarnings())
names = [w["condition"] for w in existing]
if "urban.warnings.notice" not in names:
existing.append(
{
"condition": "urban.warnings.notice",
"level": "warning",
"message": translate(
"urban.warnings.notice",
domain="urban",
target_language="fr",
default=u"Ce dossier a été encodé de manière dématérialisée",
).encode("utf-8"),
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
portal_urban.setWarnings(tuple(existing))

catalog = api.portal.get_tool("portal_catalog")
licence_brains = catalog(object_provides=IGenericLicence.__identifier__)
for licence_brain in licence_brains:
licence = licence_brain.getObject()
for event in licence.getAllEvents(IUrbanEventNotice):
if IAnnotations(event).get("notice_notification", {}):
alsoProvides(licence, ILicenceCreatedViaNoticeWS)
break
Comment on lines +754 to +761

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reindex object_provides after alsoProvides.

The migration applies the marker but never reindexes the licence. create_licence in src/Products/urban/browser/cron/notice.py calls reindexObject() after alsoProvides, so migrated licences and newly created licences end up with different catalog state. Any catalog query on object_provides=ILicenceCreatedViaNoticeWS.__identifier__ will miss the migrated licences.

Also set _p_changed to match the runtime path.

Proposed fix
         for event in licence.getAllEvents(IUrbanEventNotice):
             if IAnnotations(event).get("notice_notification", {}):
                 alsoProvides(licence, ILicenceCreatedViaNoticeWS)
+                licence._p_changed = 1
+                licence.reindexObject(idxs=["object_provides"])
                 break
📝 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
catalog = api.portal.get_tool("portal_catalog")
licence_brains = catalog(object_provides=IGenericLicence.__identifier__)
for licence_brain in licence_brains:
licence = licence_brain.getObject()
for event in licence.getAllEvents(IUrbanEventNotice):
if IAnnotations(event).get("notice_notification", {}):
alsoProvides(licence, ILicenceCreatedViaNoticeWS)
break
catalog = api.portal.get_tool("portal_catalog")
licence_brains = catalog(object_provides=IGenericLicence.__identifier__)
for licence_brain in licence_brains:
licence = licence_brain.getObject()
for event in licence.getAllEvents(IUrbanEventNotice):
if IAnnotations(event).get("notice_notification", {}):
alsoProvides(licence, ILicenceCreatedViaNoticeWS)
licence._p_changed = 1
licence.reindexObject(idxs=["object_provides"])
break
🤖 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/Products/urban/migration/update_290.py` around lines 754 - 761, After
alsoProvides in the migration loop, update the licence persistence state by
setting licence._p_changed and reindex the licence via reindexObject(), matching
the behavior in create_licence. Apply both operations only when a notice event
causes ILicenceCreatedViaNoticeWS to be added, so catalog queries include
migrated licences.


logger.info("upgrade done!")
7 changes: 7 additions & 0 deletions src/Products/urban/migration/upgrades_290.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,11 @@
handler=".update_290.setup_referenceFT_PM"
profile="Products.urban:default" />

<gs:upgradeStep
title="Add notice warning"
description=""
source="2919"
destination="2920"
handler=".update_290.add_notice_warning"
profile="Products.urban:default"/>
</configure>
2 changes: 1 addition & 1 deletion src/Products/urban/profiles/default/metadata.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0"?>
<metadata>
<version>2919</version>
<version>2920</version>
<dependencies>
<dependency>profile-Products.urban:preinstall</dependency>
</dependencies>
Expand Down