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 >> <[email protected]> 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 > [email protected] > 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
0xDC7C8BF3.asc
Description: application/pgp-keys
_______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
