Report system legacy

[David Carlson]

On Saturday, 6/29/2013 10:38 AM, Geert Janssens wrote:
> On 29-06-13 16:52, John Ralls wrote:
>> On Jun 29, 2013, at 7:06 AM, Geert Janssens
>> <janssens-geert at telenet.be> wrote:
>>
>>> Thanks all for your feedback. I'll keep from this that duplicate
>>> names among custom reports are not desired. To which I agree.
>>>
>>> I originally asked this question in the scope of one of the bugs in
>>> the GnuCash bounty program:
>>> Allow saving of Custom Reports without changing name, overwriting
>>> existing report.
>>>
>>> Before I go into more detail, let me start with this: the current
>>> code is not able to prevent duplicate custom report names either.
>>> Don't believe me ? Try this:
>>> - open a new report
>>> - edit it's name in the report options and hit ok
>>> - edit it's name again, resetting it to the original name and hit ok
>>> - save the report
>>> You can do the same thing by creating a new report and changing its
>>> name to the name of any existing custom report. There is no
>>> validation on the name.
>>>
>>> So the unique name requirement is a new requirement, which I see
>>> independently of the bounty requirement of being able to resave
>>> changes to an existing report template. I hope we can agree on this.
>>> I do intend to look into this as well, but not right now.
>>>
>>> I have been massaging the reports code for about a week now to come
>>> up with a satisfactory solution. You can not imagine what a can of
>>> worms that code is...
>>>
>>> To make the rest of the discussion a little bit manageable, allow me
>>> to first make a distinction between reports and report templates:
>>> each menu option you see in the reports menu is a report template.
>>> The moment you open one such menu you instantiate one instance of
>>> such a template. This is a report. "Custom reports" are also report
>>> templates, which have a parent template and a set of custom options.
>>> When you open a "custom report" you create an instance of one
>>> template, hence you have a report again. When you "save" a report,
>>> what you really do is creating a template based on the parent
>>> template and the current set of options for this report. In this
>>> discussion, we're constantly on this edge between report templates
>>> and reports.
>>>
>>> While looking at the custom reports code I found several issues with
>>> it, not only the inconvenience of having to change the name all the
>>> time before resaving. I came to the conclusion that simply adding a
>>> dialog box asking if a report should be overwritten is only shifting
>>> the problem to the next annoyance.
>>>
>>> Since I would be working on this code anyway, I wanted to eliminate
>>> several of these annoyances at once.
>>>
>>> So I worked from the file system metaphor that was already referred
>>> to in this discussion. Most programs have two Save options: simply
>>> Save and then Save As. You use the first to save your changes to an
>>> existing file, you use the second when you want to save your changes
>>> in another file. This is much more in line with how humans think
>>> than the current custom reports logic where you first have to change
>>> a name and only then can save it.
>>>
>>> So that's my general idea so far: make the custom reports logic more
>>> like a file manager. So far I have created two independent save
>>> buttons for reports as well: a save and a save as button which
>>> behave as you would expect: save will update the custom report
>>> template the current report is based on. Save as will prompt the
>>> user for a name and create a new template with this name. If the
>>> report to be saved is not based on any custom report template, the
>>> save button will behave as a save as button, just like a file
>>> manager save button would.
>>>
>>> The name prompt dialog is an improved version of the custom reports
>>> dialog, which also now allows to rename any existing report. Using
>>> this dialog makes it easy to see which custom report template names
>>> do exist already, so it becomes easier to generate a unique name.
>>> The old system relied on you to know which name is unique or not.
>>>
>>> Note that this solution implies that you know which template a
>>> report is instantiated from. The current code doesn't keep track of
>>> this. The obvious thing to do is to add a parameter to the report
>>> record for this. And the most obvious parameter to store is the
>>> custom report template's guid.
>>>
>>> So far a unique name is not enforced yet in my new code. But since
>>> you see the names of all existing reports when you save a new one,
>>> it's easy to maintain this manually.
>>>
>>> Now regarding this unique name enforcement, I'd like to think out
>>> lout a bit here.
>>> With the new implementation, we risk duplicate names both when a
>>> user hits "Save As..." or when she uses the custom reports dialog to
>>> rename a report. So I'll just work with the generic situation of
>>> renaming a report.
>>>
>>> Suppose I have two report templates called "TemplateA" and
>>> "TemplateB" and there are reports currently instantiated for both
>>> templates. Next I try to rename TemplateB to TemplateA. What should
>>> I do ?
>>>
>>> a. delete TemplateA and rename TemplateB to TemplateA, probably
>>> after user confirmation ? That means that old TemplateA's guid is
>>> lost, and any open report based on it is no longer based on any
>>> custom report template.
>>>
>>> You could consider updating the open reports while renaming a
>>> template, but you can't. Templates are shared across all books, so
>>> at best you can update the reports in the currently open book. The
>>> issue could still pop up in another book.
>>> An even stronger reason no to attempt updating open reports: what if
>>> TemplateA and TemplateB are not based on the same parent report ? So
>>> you now have one template "TemplateA", and some reports that claim
>>> to be instantiated from it, although they come from totally
>>> different templates originally.
>>> Each time you hit save on either report, TemplateA will effectively
>>> swap parent template in addition to options. So depending on which
>>> report you last saved, your custom template will instantiate a
>>> totally different report. Perhaps this is an uncommon situation, but
>>> it may cause lots of confusion for a user that accidentally gets
>>> into it. So I don't think it's a good idea.
>>>
>>> b. delete TemplateA, rename TemplateB to TemplateA and set
>>> TemplateB's guid to old TemplateA's guid. In this variant, suddenly
>>> all open reports that were based on TemplateB won't be based on any
>>> template anymore. This is probably a very bad idea.
>>>
>>> c. If the new name already exists, simply refuse to continue. Ask
>>> the user to change the name again. This may be the simplest to
>>> implement, but in reality this will result in situations exactly
>>> like case a, except it's more cumbersome to the user: if you refuse
>>> to overwrite a template, the user can first delete that template and
>>> then rename the new template to the old one. All issues you get in
>>> case a will repeat here.
>>>
>>> In summary, I don't think we can avoid some loss of report to
>>> template links. So which option looks the most user friendly here ?
>>> I would think option a.
>> Is there really a Report class? Isn't a Report an HTML page that
>> results from running the ReportTemplate's query, any summarizing
>> functions, and its display logic? Reports aren't saved, right? When
>> one loads a Book with open reports, isn't the
>> ReportTemplate retrieved and the Report HTML regenerated?
> Hi John,
>
> Thanks for your feedback.
>
> Just for clarity, the definitions of report-template and report are
> purely in guile code (src/reports/report-system/report.scm). Our c
> code is totally unaware of this. It just passes around opaque SCM
> objects. I don't even know where the html page appears in this
> picture. It's not in this part of the code yet. A "report" in
> report.scm is defined as a record containing a report-type (which is
> the guid of a base report template), a report-id (just a number), a
> structure holding the report options, a reference to the editor widget
> and some housekeeping flags.
>>
>> Where I'm going with this is that if a report is if a Report is open
>> it will just sit there until the user tries to reload it (which would
>> include loading a Book with an open report). If the ReportTemplate is
>> changed, the Report gets re-created based on the new ReportTemplate,
>> right? If the ReportTemplate is gone, which is currently possible if
>> one deletes a "custom report" in the dialog box, I would hope that
>> Gnucash displays a nice "Can't find the Report" in the HTML page
>> rather than crashing.
> This doesn't happen, because the custom report's template id is never
> stored in a report record. Instead the base template on which the
> custom report template is based on is stored in the report record. So
> when you reopen a report (by reopening a book), the report currently
> doesn't even know it ever was instantiated from a custom report. It
> will reinstantiate from the base template. I believe this was
> originally done exactly to prevent unexpected crashes when the custom
> template got deleted.
>
> This results in a two-level template hierarchy. The base templates are
> all the report definitions that ship with gnucash, or that a user
> loads manually (what I would consider a true "custom report"). What is
> called custom reports now are the reports a user can save from within
> the program. Which really aren't much more than a reference to a base
> report and a custom set of default options. The code keeps this
> hierarchy shallow. There will never be a custom template based on
> another custom template. Which I think is good exactly to avoid the
> template is deleted issue.
>
> But I needed to keep track of which custom template a report is
> instantiated from, so I have now added an extra parameter in the
> report record to keep track of which custom report was optionally
> instantiated when the report was generated. I'm still keeping this
> parameter separately from the "report-type" which will always hold the
> core template to prevent the crashes you describe.
>
> If the custom report gets deleted, the reference to it in the report
> record becomes invalid. But this is still no problem, because the rest
> of the code will treat an invalid reference as no reference at all. Or
> put another way: if you delete a custom template, any report that was
> instantiated from it will now be treated as if it was instantiated
> from the custom template's parent (or base) template and everything
> continues to work.
>>
>> So for your rename problem, don't allow renaming TemplateB to
>> TemplateA: Require the user to explicitly delete TemplateA. Don't
>> change the GUID for TemplateB, and retrieve report specs by GUID
>> rather than name. The behavior for open Reports based on TemplateA is
>> then the same as it is now (unless it crashes Gnucash, which should
>> be fixed first).
>>
>> Regards,
>> John Ralls
>>
>>
>>
> Given how things currently work I think it's even ok to allow renaming
> TemplateB to TemplateA, added a proper warning, explanation and user
> confirmation. It will have the exact same effect as  having the user
> delete TemplateA and then rename TemplateB to TemplateA in two
> separate steps. Other than that I agree that guid should be used as
> key. That's already the case now.
>
> Geert
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
I didn't think that I had anything to add to this discussion, but while
I was reading these last three items I was reminded that one of my pet
peeves with reports (not just in GnuCash) is both the lack of
availability of a complete auditor friendly list of the settings used to
generate that particular report and possibly an audit trail of changes
to the report settings when a report is generated from some sort of
template that has been used before. 

Would it be possible have an option to print with a given report a page
or two appendix listing the report settings?

David C
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0xDC7C8BF3.asc
Type: application/pgp-keys
Size: 1729 bytes
Desc: not available
URL: <http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20130629/62acf9ae/attachment.bin>