Option to rename the file before downloading

changed the description

@shahrukh_compucorp does adding the ability to name the file on the form meet your need? When I first read this that was what I thought it would be & it seemed useful more broadly

@eileen thank you for reading and responding back yeah as far as my needs are concerned that would be sufficient but it would be needed to add to all the forms that are responsible for file downloads.

Hi @eileen jumping in here to add on from Shahrukh. Thanks for your quick response.

I completely agree with you that having this functionality across forms would be ideal. We are on a slightly tight time frame currently, with a big go-live for this particular feature. Could we start small with this (doing the form on email suggest by Shahrukh in the PR) and aim to expand this functionality across CiviCRM once we know it works and is approved by the client? That way we can get this out for the go-live and plan in the following work needed?

Thanks! Fran

@fran so the proposal being to add a field to the email form permitting the field name to be specified & using that if it is?

I think that's OK- and I'd rather focus on a single form first.

Some sort of validation would be needed for security I expect - probably restricting to alphanumeric + . - or _

(ping @seamuslee @pfigel )

Yes, exactly @eileen - we will focus on 1 form first, adding a field to the PDF* form allowing for file re-naming. We'll test this with the client and then all going well we can add this field / file download functionality across civicrm where makes sense.

@shahrukh_compucorp please include this validation. File name can only be alphanumeric plus . - and _ (please in addition add a blue 'help' button at the end of the field to explain this).

Thanks!

There are validation rules in CRM_Utils_Rule - if there isn't one that is already useable then it may make sense to add another

@shahrukh_compucorp @fran also be careful with the email form as the CRM_Contact_Form_Task_EmailCommon might be in use in older versions but is deprecated in favour of CRM_Contact_Form_Task_EmailTrait on master

Apologies @eileen , it's the print/merge document (PDF) form we are working on. I hope that is slightly less risky than email, but is there anything we should be careful with regarding PDFs?

I don't think so - I'm trying to clean up those forms a bit - but slowly - I doubt it will impact

There is one PR open on aa variant of that form https://github.com/civicrm/civicrm-core/pull/18634

Ah yes. I think they are similar (we've also been thinking about a few things with print/merge documentation so great to hear you're clearing them up too). Shahrukh will cross check the Email form anyway to make sure.

So far I'mm mostly just chipping of little bits of tidy up like https://github.com/civicrm/civicrm-core/pull/18768 (which is in the parent class)

Little by little is the way to go! We're hoping to focus more on clearing up after some big go-lives coming up shortly.

Is there anything else regarding this PR? Otherwise we can crack on and hope it gets good feedback!

@fran @eileen my understanding is that this would deal with changing the filename that is provided in the Content-Disposition HTTP header in various places, rather than actually changing existing file names in civicrm_file and on disk, correct?

I can't think of any security concerns in that case. Header injection in header() is mitigated by PHP itself, and nothing else comes to mind.

I did think a bit about whether a more generic alterHttpHeader hook or similar (basically applying the idea behind hook_civicrm_pageRun to HTTP headers) might make more sense, but no strong preference on that. This is probably more intuitive.

@pfigel yeah actually it will only be renaming the file that is getting downloaded once you click on the download button in Create pdf forms or print merge documents.

Cool - go for it

added git:civicrm-core ~88 needs:concept approval triaged labels

Hi @eileen can we get the magic "concept approved" tag just for clarity? Thanks!

added Concept approved label and removed needs:concept approval label

Hi @eileen @pfigel

We did some quick UX testing for this feature with the proposed approach of having an additional field on the form and we had some quite negative feedback that it was confusing for users.

They raised the following points:

Users are clicking on the "download document" button without filling in the field as they are not near each other. As such they then get a validation message that the name is required which they don't like.
We do not save the contents for the field, it is only used when you download the final document.
They are confused as to why the filename is not used for preview given that it is shown always.

As such what we would like to suggest is that we have a popup that shows after the user clicks "download document" in order to allow the user to enter the file name. This is similar I guess to how MS Word would have a save screen which asks for the filename when you save the file for download.

Please see mockup below:

I'm aware that we don't have many examples of this kind of experience in Civi, but I really think this would be the best option. Would you be happy with this?

If we can get a yay or nay quickly that would really help as it appears we actually need to finalise this today

I hate to block things, but this seems to me like an odd pattern. We are piling of top of CiviCRM's odd habit of asking geeky questions to users who just want to get their work done.

I agree that CiviLetter.pdf is a bit ridiculous, and we should offer a sane default. The civiexportexcel extension does that for reports (defaults to generating a file based on the report title, timestamp, etc.)

For example, instead of CiviLetter, it could be "letter_[date]_[cid/display_name]".

Having a hook on top of that also makes sense, but it only helps a handful of developers, not 90% of our users, so we should also think about fixing the default. The hook might also need extra context, in order to be useful (entity type, entity id, context = letter/other).

Why? Browsers (at least Chrome & Firefox) offer this as a standard feature configured by user preferences.

@ayduns @bgm

Guys, this isn't really fair...

We've had the concept approved over a week ago and 20h work has already gone into this - against a tight client deadline (today in fact we need to get the release out). I'm sorry but it's not really fair to start a new thread of comments late and after that time. We've just had to have another 8am call to align 2 devs / QA and PM on what to do here and to manage our release cycle. Its things like this that makes it impossible to plan any work if anything ever needs any core improvement...

Also I think that the point of the ticket is being missed.

The key point is not the name of the file that is downloaded to your Computer, a user can obviously always change this after it is downloaded. The problem is that CiviCRM creates an activity with the file in the activity listing and the filename of the file that is created automatically by CiviCRM is also named "CiviLetter.pdf". This makes it impossible for a user to know what the file was once it was created. There is no way for the user to change this.

With respect to having a better default filename - on the activity we already have the date and time of person creating it. This doesn't solve the attachment name issue which will always be generic.

As such I suggest the following:

We are about to submit a PR for the backend for this work which will allow the system to accept a parameter for the filename. This will have no impact on the user.
We will wait on doing the front end work until we get a consensus on what to show to the user. For the time being we will carry that as a customisation in the CiviCase extension so core is unaffected.

I hope that meets everyones requirements.

Thanks

J

changed the description

@jamienovick1 - you changed what you're proposing just yesterday ...

The UI only...

"With respect to having a better default filename - on the activity we already have the date and time of person creating it. This doesn't solve the attachment name issue which will always be generic."

The updated initial proposed behaviour makes sense and seems to do that (check a special field name, that can be used instead of CiviLetter.pdf).

Then an extension can alter it with a buildForm hook?

I understand you are under pressure from deadlines. I think it's better if we stick to a technical arguments. One person's feature is another person's bug, and I appreciate that you were able to quickly find a better solution.

It would be very helpful to have the filename exposed, and therefore to be able to write an extension to customize the file name. I also care about the downloaded file name - yes, it can be renamed but that's extra work and it adds up if you are downloading (and keeping track of) multiple files. It's also a great opportunity for errors and sending the wrong file to someone.

I understand the behavior mentioned in this thread is not, in fact, in CiviCase anymore.

Would Core be open to a patch to enable customization in extensions?

Hi @ananelson just obtained a bit more information on this. We did work on this as a customisation to the new Civicase extension that you left a comment on, but this has not been merged to the main extension. Could you pop me an email at jamie@compucorp.co.uk and perhaps we can discuss how we can get this merged to the main CiviCase extension? Best

Also this link never got posted here: https://github.com/civicrm/civicrm-core/pull/18938

@jamienovick1 Has there been any movement on this since the comment on 18 June about a new PR coming? Thanks

@agilewarefj Yes this is still in progress, and we are trying to send the final PR within next couple of weeks.

mentioned in commit 9d867a27

changed milestone to %5.42.0

https://github.com/civicrm/civicrm-core/pull/21006 has been merged. Closing. Thank you @debarshi.bhaumik ! (and the many people involved in the design & review)

closed

mentioned in issue #3068 (closed)

Option to rename the file before downloading

Overview

Problem statement

Example use-case

Current behaviour

Proposed behaviour

Child items ...

Activity