Modify

Opened 4 years ago

Last modified 20 months ago

#7759 new task

Update Account Manager Announcer for new subscription system

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

Description (last modified by rjollos)

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 rjollos 21 months ago.
Patch against r12352 of the trunk

Download all attachments as: .zip

Change History (13)

comment:1 Changed 3 years ago by rjollos

  • Cc hasienda added
  • 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 22 months ago by hasienda

  • Cc hasienda removed
  • Keywords acct_mgr subscriber added; 0.12 removed
  • Owner changed from doki_pen to hasienda
  • Priority changed from low to normal
  • Summary changed from Update Account Manager Announcer for new system to Update Account Manager Announcer for new subscription system
  • Type changed from defect to task

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 22 months ago by hasienda

(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 22 months ago by hasienda

(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 22 months ago by hasienda

(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 22 months ago by hasienda

(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 21 months ago by rjollos

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 21 months ago by rjollos

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

comment:9 Changed 21 months ago by rjollos

[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 21 months ago by rjollos

Patch against r12352 of the trunk

comment:10 Changed 21 months ago by hasienda

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 21 months ago by hasienda

(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 20 months ago by hasienda

(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.

Add Comment

Modify Ticket

Action
as new .
Author


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

 
Note: See TracTickets for help on using tickets.