The desktop client slugify the filename but I do not know how the web browser manage them (but I guess they handle it).
1 Like
So there is consensus that custom report names would be a nice feature 
I think that the class for which the report is registered should have a method?
def get_report_name(cls, records, report_name):
if len(records) == 1:
return records[0].rec_name
else:
return '{}_{}'.format(report_name, len(records))
is what I did in my installation. What I don’t like is getting this method by records[0].__class__ aquisition.
I do not think we need an new method, the execute method could contain this kind of code. Indeed such design is already used to name file inside zip.
Mmhh. If I customize a class it seems more easy to me to rewrite the report name in a method than customizing all the excecute of the report by subclassing, registering etc. Or if it is bad design to do it in the class of the records there should be a dedicated method of the report class.
In any way, you must extend the report class. But as the report name is part of the returned value, for me we do not need to blow up the Report class.
I agree that we do not need a new method. Currently its easy to extend a report name. Here is an example:
@classmethod
def execute(cls, ids, data):
pool = Pool()
Product = pool.get('product.product')
result = super().execute(ids, data)
if len(ids) == 1:
product, = Product.browse(ids)
if product.name:
result = result[:3] + (product.name,)
return result
I remember having issues with zip files when the sequence contained the / character. And probably this is related to both clients.
The filename in zip file must be slugified: File name in zipfile should be slugify (#9203) · Issues · Tryton / Tryton · GitLab
This is a lot of knowledge one need to have about the internals of the report. And you need to now how to register this patched report for your document class. I can do it, but I think for newcomers it is cumbersome. If this is considered the way to go, we should write a bit of documentation.
I’ve tested and yes the browser handles the presence of / in filenames…
Agree that adding some documentation on how to do it (with an example) will be also a good improvement.
So if you want to work on it, fill two issues: one for documentation an another to include the rec_name of the records in the name by default.
But I think customization comes second if we have a good default behavior. So having the record name seems the most expected behavior for single record. For multiple records, it is not clear for me what is the best solution. Because when there are few records, we could list their names (separeted by -) which is more meaningful. Maybe we could list the 5 first and add + (I prefer to avoid ...) like in some existing warnings.
1 Like
How about adding two fields to ir_action_report:
- report_name
- zip_report_name
and have those set to a sensible default but having the user be able to write their own PYSON in there (and of course slugify that - preferrably when saving the report settings/data - and throw an error if it contains invalid chars (’/’, ‘’, ‘:’ , …) – need to slugify again later when generating anyway since you could use various DB fields to get the values which could in turn contain invalid chars but that should be easy.
If this is something that (from a workflow/… point of view) would be considered for inclusion in the standard tryton source I can write a patch for this and submit it, but let’s discuss first if you think this is feasible (I do think it is definitely better than having to subclass Report just to change file name(s) … be that by having a method to override or by customizing execute)
For me we can not manage to template the report name because reports are based on list of records so there is no direct variable that can be used. For the zip name, it is even worst because it must be unique for any record and this can not be checked with a template.
Finally Issue 9509: Report could take the name from the record(s) rec_name - Tryton issue tracker improved the naming in a generic way that avoid the need of customization for the majority.
True i did not think of the “list of records” aspect … that doesn’t make it easier.
Anway while I do not need the report names to be changed, currently the users change the names manually when saving, so it would save some time to have an easy way to customize …
anyway while a method to get the name might make customization easier I realise this is something probably not needed by many so might be overkill… guess I will use something like the execute() code that pokoli posted in April last year – since I need to create my own report class now anyway for other reasons
Maybe we can use the record variable to template and call it for each record on the list.
If not template is set we may keep the current behaviour (using the rec_name) of the record.
In this case I think we can follow the same patter but add the id at the end. To make it unique.
Currently the id is added at the start and we get some complaints becausae the order of the filenames is lost when using the zip folder.
Another problem with zip files is that the name can not be customized without altering the rec_name.
That’s some feedback we received recently from a customer that wanted to get the PDF of multiple invoices in a zip file.
That’s the current option for now.
Why is record name not good enough? There should be a very strong and large need to introduce a new configuration variable.
Maybe. It should be tested with long similar names they do not conflict.
I see no problem with that.
To be honest … I am currently contemplating if I can/should just customize rec_name in our case… it might be a feasible option for this report at least … but I am unsure where rec_name shows up / is used otherwise so I need to check
For me I care the most about the name of the invoices and sale orders, because I’m sending them via email and have now change the name manually. The rec_name is already a very good improvement so keep that.
As an extra I can imagine an extra field (‘customize report name’) on the report record. This field is only active when ‘Single’ is checked, so there is just one record on the report. The field behaves exactly the same as the ‘subject’ field of the (new) email template (I love it!), so you can add for example Invoice ${record.number}. The name of the report becomes then ‘Invoice 202134929’ or whatever number the invoice has.
Together with the new email templates and sending email from Tryton that will be a nice addition.
I agree that rec_name is a quite good default, but … here is my biggest issue with that:
I have more than one report from the sale module … one for pre-production (internal) documentation and then I will also have another one for the actual sale later (unless I use invoice directly there …)
so being able to configure that on the Report record would be definitely a great option as I might not want every report for each type to have the same name.
Though I have no idea how common it is across the user base, that there are different reports available, that should ideally have different names.
like I want my project documentation to have the name different from the actual Sale docs / invoice (i know invoice is account.invoice of course so that is seperate)
To stay with the invoice example, you can even add the state of the invoice to the report name. So you have a clear distinction.
I have also multiple reports for invoice or sale. Because when I’m sending the report by email I want to have it as PDF and with my company letterhead as background. For printing I don’t want to have a letterhead.
As I look into the patch https://codereview.tryton.org/306161002/patch/300481002/306461003 It seems to me that the report name will be still the field name from the report template but also including the rec_name from the first 5 records.
So to sum it with an example:
name field on the report template is ‘Default Invoice’
rec_name of the record is ‘INV392890 [OR342432]’
You report name will be Default Invoice-INV392890 [OR342432]
I think what I will do is, is implement this on my custom class and see if it works or crashes 
– If it works I’ll post a link to it here for either further discussion or future reference for people with similar requirements