in issue 2551228 I noted:

   It addresses the need to  compare values of interesting properties in the reactor
   rather than checking to see if the properties we are interested in are "in" oldvalues.
   Currently:

      "interesting_prop" in oldvalues

   is always true even if interesting_prop wasn't changed in this
   transaction.

   The spec (references.txt) for reactors says olddata (usually named
   oldvalues in actual code) says: 

     For a ``set()`` operation, ``olddata`` contains the names and previous
     values of properties that were changed.

   oldvalues has the previous value for *all* properties of the object, not
   just changed properties. This differs from an auditor where "'propname' in
   newdata" is a useful statement. I'll open a discussion on the dev list. If
   this is a mistake (it has been this way since version 0.5.0 AFAICT) I'll open
   a ticket to fix it in the core and reference it in this issue.

The discussion starting with: https://sourceforge.net/p/roundup/mailman/roundup-
users/thread/CANfx4mvKAh4Qgju8-
AgJ8Y0ZRAomeydgsWdTYXL5iCM6CZnJ7A%40mail.gmail.com/#msg59407652

leads me to think it is a bug (Ralf noted the issue a while ago, but wasn't sure if
it was intentional' he also compares values to see if a value changed).

If you use oldvalues.get('propname') you get None returned if the keyword is missing.
If you use oldvalues['propname'] you get a KeyError raised.

Ralf uses .get() in his reactors.

So three options to fix this:

  1) update the docs to indicate the current code
  2) change the code to only include changed database items and everybody needs
     to audit their reactors.
  3) provide two paths with/without unchanged props and a switch (in
     config.ini or interfaces.py) to remove unchanged props (default is
     to include them).

1 is easiest, but I really want "'unchanged prop' in oldvalues" to
work because it reduces cognitive load by matching auditors and
eliminates unneeded database accesses to compare values.

2 is preferred but makes upgrading more difficult.

3 adds another switch that most people won't know about, clutters
config.ini and increases testing burden.

Ralf voted for 2 with docs in upgrading.txt. He expects his reactors all use oldvalues
and a grep should allow him to identify cases where there is a problem.

Unless other people chime in, I'll start work on 2 tomorrow.

The work includes:

  code changes to strip unchanged items

  check packaged reactors do not break with this change

  upgrading.txt doc entry at 'required' level

  doc checks for any reactors to make sure they use 'propname' in oldvalues
      or "o_val = oldvalues.get(); if o_val is None: return" etc. for short circuiting.


  check wiki for problems and update pages with a notice about this change.

I think that's everything to cover.

Blech. ## marks my comments and are not in code.

In roundup/roundupdb.py generateChangeNote() we have this code:

          for key in oldvalues:
            ## [... ]
            # the old value might be non existent
            # this happens when property was added
            try:
                old_value = oldvalues[key]
                if isinstance(new_value, list):
                    new_value.sort()
                    old_value.sort()
                if new_value != old_value:
                    changed[key] = old_value
            except Exception:
                changed[key] = new_value

We have set() in hyperdb.py and a couple of other places where getprops
is iterated over
for a similar purpose. So I think this is valid.

Before the loop I have:

        props = cl.getprops(protected=0)

I think I can change:

          for key in oldvalues:

to

          for key in props:

and eliminate these statements inside the loop:

        if key in ('actor', 'activity', 'creator', 'creation'):
                continue
            # not all keys from oldvalues might be available in database
            # this happens when property was deleted
            try:
                new_value = cl.get(issueid, key)
            except KeyError:
                continue

since deleted and protected props won't be in props.

However I need to add the assignment of new_value to:

            # the old value might be non existent
            # this happens when property was added
            try:
                new_value = cl.get(issueid, key)  ## add this since key
is always a valid prop
                ## can raise KeyError
                old_value = oldvalues[key]
                if isinstance(new_value, list):
                    new_value.sort()
                    ## could be a non-list, but this would be a change
of type for the property
                    ## which is not allowed. However it could raise
ArttributeError
                    old_value.sort() 
                if new_value != old_value:
                    changed[key] = old_value
            except Exception:  ## change to (AttributeError, KeyError) 
                changed[key] = new_value

which should be safe since only props present in the db are being
iterated over.
It also cleans up a bare Exception handler.

Also nosymessage() used with note_filter(..., oldvalues) needs to be
audited by
the tracker admin and added to updating doc.

Ralf any thoughts?

Hi Ralf:

Pushed this branch to mercurial on sourceforge:

  https://sourceforge.net/p/roundup/code/ci/
reactor_pass_changed_oldvalues/tree/

can you take a look and see if it looks sane?

It has the possibly undesired side effect of removing quiet
parameters from oldvalues.

The commit message has details on how we can restructure so that
the quiet params are filtered on return from set(), but
made available to the reactors.

With the old way, you could see the prior values for quiet changes
if you wanted to do something with them, but you had to do your own
comparison to see if the parameter had changed.

In theory you can look at the journal for the quiet changes by passing
a flag to prevent filtering of the quiet changes provided the parameter
has journaling enabled.

-- rouilj

Hi Ralf:

> It has the possibly undesired side effect of removing quiet
> parameters from oldvalues.
>
> The commit message has details on how we can restructure so that
> the quiet params are filtered on return from set(), but
> made available to the reactors.

I added changeset:   8833:36852e27e1a1 to extract quiet parameter
filtering from set_inner
and moved it to the parent set() method. I apply it after the reactors
are called
before set() returns and provides the changed properties to the
confirmation method.

Comments?

On Mon, Oct 12, 2026 at 03:21:05AM +0000, John Rouillard wrote:

> I added changeset:   8833:36852e27e1a1 to extract quiet parameter
> filtering from set_inner
> and moved it to the parent set() method. I apply it after the reactors
> are called
> before set() returns and provides the changed properties to the
> confirmation method.

Look good, I wasn't aware that something like quiet props exist :-)

Thanks!
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

> Look good, I wasn't aware that something like quiet props exist :-)

I added it a while ago.

I have an issue template with multiple fieldsets. The user can collapse/
open them.
The the state of collapsed/open fieldsets is sticky. When the user saves
the issue form (even without changing any issue data), the collapsed/
open
state data is saved to a property on the user object.

When the user looks at any issue, the open/collapsed states are
recreated.

However notifying the user that the underlying state property was
changed is just noise
and not user facing. Hence quiet suppresses notifications for updates to
that field.