Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow developers to choose attachments #4477

Merged
merged 4 commits into from Dec 6, 2017
Merged

Conversation

ci2014
Copy link
Contributor

@ci2014 ci2014 commented Nov 13, 2017

This change allows developers to choose attachments, even if there is a cur_frm object, like so:

new frappe.views.CommunicationComposer({
	doc: frm.doc,
	frm: frm,
	subject: "Hello",
	attachments: [
		{
			file_name: "filename.jpg",
			file_url: "/files/filename.jpg",
			is_private: 0,
			name: "a50177b946"
		}
	]
});

Without this change the attachments of the document are used instead of the hard coded attachments, which obviously isn't what developers want.

Fixed and manually tested

This change allows developers to choose attachments, even if there is a cur_frm object.
@rmehta
Copy link
Member

rmehta commented Nov 13, 2017

Please share a use case for this.

https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it

Please add animated GIF or test case.
https://github.com/frappe/erpnext/wiki/Contribution-Guidelines

@ci2014
Copy link
Contributor Author

ci2014 commented Nov 13, 2017

@rmehta Developers will need it, when they want to send a specific attachment with a certain kind of email, not all attachments (if there are any) of the current doc (might be useful for attaching terms and conditions).

However this was supposed to work (look at the current code), but the if-else-clause was just the wrong way. A hard coded attachment object (this.attachments) should have priority over the attachments in the current doc.

Lets assume, that we have a docfield "button" with the label "Send Terms and Conditions to customer", which opens a mail form, while we are in a doc with 5 random attachments, but we only want to send the Terms and Conditions pdf from our file manager to the customer. Developers should have the ability to do so:

attachments

@rmehta
Copy link
Member

rmehta commented Nov 13, 2017

Why is this not built as a generic feature?

@ci2014
Copy link
Contributor Author

ci2014 commented Nov 13, 2017

@rmehta You mean something like automatic default attachments? They would require this PR. Then we could build something like automatic attachments.

I also think about automatic assignments for DocTypes based on rules, but thats another topic.

@netchampfaris
Copy link
Contributor

@ci2014 Hey, why not add both? The attachments in the frm as well as passed through this.attachments. Anyway, the user has to check the attachments she wants to send.

@ci2014
Copy link
Contributor Author

ci2014 commented Dec 6, 2017

@netchampfaris Good idea. The user will be more flexible in choosing his attachments. Next up would be for the developer to pre-check some of the attachments. I already have this, but it would need some refactoring since it repeats some code.

@netchampfaris netchampfaris merged commit 01a955e into frappe:develop Dec 6, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants