Comment 3 for bug 1206589

The database table and IDL object are config.org_unit_setting_type_log
and coustl. These should probably be actor.org_unit_setting_log and
aousl, respectively. It may be too late to change this without
inconveniencing someone, but I suggest changing the table name and the
IDL.

Because the table and IDL seem to be based on coust, the IDL uses
permissions based on ADMIN_ORG_UNIT_SETTING_TYPE, and a new permission
for managing the log: ADMIN_ORG_UNIT_SETTING_TYPE_LOG. Given that
we're actually tracking changes to the settings themselves, and not to
the setting types, these permissions should be changed, or even
dropped. Log entries should be created and managed in the backend
without regard to the user's permissions. Log entries should only be
viewable to users who have the view permission for the referenced
setting at the referenced organizational unit. (More on how the
history is exposed appears later.)

The IDL lists open-ils.pcrud as a controller. This listing is
problematic since org. unit settings do not follow the pcrud model for
permissions. The open-ils.pcrud controller should be dropped. The
parent aous permissions should be used by log entry API calls. Given
that open-ils.cstore would be the remaining controller, the backend
can still manage the log without needing permissions.

The log records the date of the change, the org unit where the setting
applies, the original value, the new value, and the setting (field
name) that was changed. It does not track what user made the change,
nor at what workstation. I suggest adding the user and workstation as
new columns if possible.

The database trigger, limit_log_oust, limits the settings log to the
five most recent changes for any setting determined solely by the
field name. The limit of 5 is "hard coded" by using a combination of
dates and a limit of 4 on a subselect in a larger delete statement.
This limit should, itself, have a setting, either an internal or
global flag, or an org. unit setting itself. Furthermore, the limit
code should be expanded to look at both the org_unit and the
field_name, or if the table/IDL changes suggested above are adopted,
the parent aous. While this will expand the logs a bit, it will give
each site its own history. If an org. unit setting is the method of
controlling the limit, it also gives each location the chance to
control how much of the history they can see.

The staff client uses the openils.PermaCRUD JavaScript module to
retrieve information to display to the user. As mentioned above,
pcrud is the wrong permission model for org. unit settings.
Additionally, the IDL specifies an empy retrieve permission, which
means that none is required to view the history of any setting. This
not only exposes settings in the staff client, but anyone who can
currently login and make the appropriate open-ils.pcrud call remotely
could also gain access to the history of a given setting or settings.

Additionally, the pcrud search is done only on the setting field name
itself. Thus all settings are exposed in the history view and not
just those that concern the currently logged in user. It makes sense
to me that the retrieval should limit itself to those org. unit's in
the user's branch of the hierarchy, i.e. ancestors and/or descendants.
Workstation org. unit should be preferred over home org. unit for
this.

Since I have suggested removing the open-ils.pcrud controller, leaving
cstore as the only mode of access to these settings, new API calls
would need to be added to search and retrieve the settings history. I
don't think we'd want update or delete as those operations are handled
in the database.

Any feedback is welcome.