Modify

Opened 7 years ago

Closed 4 weeks ago

#7759 closed task (wontfix)

Update Account Manager Announcer for new subscription system

Reported by: Robert Corsaro Owned by:
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords: acct_mgr subscriber
Cc: Trac Release: 0.12

Description (last modified by Ryan J Ollos)

Just as the summary says. Look at FullBlog example in /announcer/opt/fullblog.py for an example.

Attachments (1)

t7759-r12352-1.patch (1.9 KB) - added by Ryan J Ollos 4 years ago.
Patch against r12352 of the trunk

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added; anonymous removed
Description: modified (diff)

I need to get this working since I just started working with the AccountManagerPlugin on Trac 0.12.2. Related tickets are #7974 and #7977. Perhaps will find some time this weekend ...

comment:2 Changed 5 years ago by Steffen Hoffmann

Cc: anonymous added; Steffen Hoffmann removed
Keywords: acct_mgr subscriber added; 0.12 removed
Owner: changed from Robert Corsaro to Steffen Hoffmann
Priority: lownormal
Summary: Update Account Manager Announcer for new systemUpdate Account Manager Announcer for new subscription system
Type: defecttask

Well, guess we all know, that it'll require still a bit more time. And certainly not due to being lazy or ignorant.

Busy times. Let's work and at least test together, as time permits.

comment:3 Changed 4 years ago by Steffen Hoffmann

(In [12312]) TracAnnouncer: Update AccountManagerPlugin messaging support, refs #7759, #7977, #8740, #8927, #9090 and #9204.

This long-standing regression is fixed now, while associated message templates are rather bare-bone ones yet and formatting could be improved significantly.

comment:4 Changed 4 years ago by Steffen Hoffmann

(In [12325]) TracAnnouncer: Fix generator, that was broken by [12309], refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

And the same bad filter code even got replicated in [12312]. Sorry for not checking compiler errors earlier. Finally I discovered an UnboundLocalError for resource_id hidden behind the first error. Obviously unit tests are a blessing and needed here too.

comment:5 Changed 4 years ago by Steffen Hoffmann

(In [12331]) TracAnnouncer: Really fix filter now, refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

Complete the change from [12325] to get expected behavior, or filters would be applied undesirably.

comment:6 Changed 4 years ago by Steffen Hoffmann

(In [12342]) TracAnnouncer: Add 'acct_mgr' as default for 'filter_exception_realms' option, refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

IMHO this is required for better plugin usability, making AccountManagerPlugin notifications pass without additional configuration effort now.

Some Python doc-string tweaks and another unit test slipped in here too.

comment:7 Changed 4 years ago by Ryan J Ollos

I'm seeing a syntax error from [12342]. I hope you don't mind if I push a quick fix now.

04:03:56 PM Trac[loader] ERROR: Skipping "announcer.filters = announcer.filters": 
Traceback (most recent call last):
  File "/home/user/Workspace/th10584/trac-trunk/trac/loader.py", line 68, in _load_eggs
    entry.load(require=True)
  File "/home/user/Workspace/th10584/lib/python2.6/site-packages/distribute-0.6.24-py2.6.egg/pkg_resources.py", line 1989, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File "/home/user/Workspace/trac-hacks/announcerplugin/trunk/announcer/filters.py", line 51
     def filter_subscriptions(self, event, subscriptions):
       ^
SyntaxError: invalid syntax (filters.py, line 51)

Having made little errors like this myself makes me really appreciate the value of CI, which I enjoy at my day job and hope we can make available to trac-hacks someday.

comment:8 Changed 4 years ago by Ryan J Ollos

(In [12353]) Refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204: Fixed minor syntax error introduced in [12342].

comment:9 Changed 4 years ago by Ryan J Ollos

[12353] was just a quick fix, but I'd suggest we also add a basic unit test. A very simple patch which adds a unit test that would fail prior to [12353] can be found in t7759-r12352-1.patch.

Changed 4 years ago by Ryan J Ollos

Attachment: t7759-r12352-1.patch added

Patch against r12352 of the trunk

comment:10 Changed 4 years ago by Steffen Hoffmann

Obviously the change has been a bit too fast and focused at 'just' getting AcctMgr notifications fix done. Indeed, I was supposed to not let unit testing slip out of sight too much. Thanks a lot for reminding me.

comment:11 Changed 4 years ago by Steffen Hoffmann

(In [12357]) TracAnnouncer: Add a bare-bones filter unit test, refs #7759.

Thanks to Ryan J Ollos for nudging me about it.

33 unit tests pass now for Trac-0.12 and above, two of them fail for Trac-0.11 for known db API incompatibility issues, that will be addressed soon.

comment:12 Changed 4 years ago by Steffen Hoffmann

(In [12503]) AnnouncerPlugin: Extend AccountManager notifications as required, refs #843, #7759 and #7977.

Note, that any previous version of TracAnnouncer won't work with latest AccountManagerPlugin 'trunk' code, and this already made me thinking about a more robust change listener definition. But this is another subject.

comment:13 Changed 5 months ago by Ryan J Ollos

Owner: Steffen Hoffmann deleted

comment:14 Changed 4 weeks ago by Ryan J Ollos

Resolution: wontfix
Status: newclosed

See #13124.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.