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

feat(shortcode): Add Company attribute to give_receipt shortcode #3361

Closed
mathetos opened this issue Jun 13, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@mathetos
Copy link
Member

commented Jun 13, 2018

User Story

As a Give Admin I want to be able to display the Company field on the Donation Receipt page.

Current Behavior

Currently, the Company Name is not added by default to the receipt, and there's no option to add it without code.

Expected Behavior

I expect that the Company field would be included in the output of the give_receipt shortcode by default, and that I'd have the option in the Shortcode Builder to exclude it optionally:

image

Related

Tasks

  • Ensure the company field is output (if present) in the output of the Donation Receipt by default
  • Add the Company field as an option in the Shortcode Builder with "Include" as the default value, with an additional option to "Exclude" it.
  • Ensure that the output of the donation receipt respects the new shortcode attribute when it's set to exclude the company field.
@mathetos

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2018

UPDATE:
I do see that the code to pass the value is in the shortcode, but it has to be passed as company_name="true" in order to display, and as shown above, that's not discoverable in the builder. So the only task is to have it output by default (if present) and show in the builder).

@ravinderk ravinderk assigned ravinderk and unassigned raftaar1191 Jun 14, 2018

ravinderk added a commit that referenced this issue Jun 14, 2018

Merge pull request #3364 from /issues/3361
feat(shortcode): Add Company attribute to give_receipt shortcode #3361
@raftaar1191

This comment has been minimized.

Copy link
Member

commented Jun 14, 2018

@ravinderk Here is the point that needs to be cover but the first two-point is not done as expected. The third point is working fine

  • Ensure the company field is output (if present) in the output of the Donation Receipt by default
  • Add the Company field as an option in the Shortcode Builder with "Include" as the default value, with an additional option to "Exclude" it.
  • Ensure that the output of the donation receipt respects the new shortcode attribute when it's set to exclude the company field.

@raftaar1191 raftaar1191 reopened this Jun 14, 2018

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

@mathetos @raftaar1191

We can not show company name by default because Shortcode builder do not generate attribute value for default value and it will be difficult to add backward compatibility for existing receipt shortcode. .

@ravinderk ravinderk closed this Jun 14, 2018

@mathetos

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2018

@ravinderk I think I understand. Essentially, we currently only have a global setting for the Company option, not a per-form setting. So your PR essentially just ensures that the Company field is output in the Donation Receipt by default, no changes necessary to the shortcode at all.

But to be clear, I was referring to the Shortcode Builder for the give_receipt shortcode, not the give_form shortcode. So we're not talking about per-form functionality when it comes to the Builder. Instead, the Builder is used for customizing the give_receipt shortcode. This would respect the global setting just fine. I'm simply saying that because we have the option in the shortcode, we should expose that in the Builder.

Does that make sense?

@mathetos mathetos reopened this Jun 14, 2018

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

@mathetos We have per form company field setting:

image

@mathetos Shortcode builder does not allow us to set default value other than the default value of shortcode attribute. That means by default company_name has false value, so we can not set it to true in shortcode by default. Admin has to choose that value and then it will be set in the shortcode.

@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Jun 15, 2018

Slack Chat Summary

Participants: Matt Ravinder
Topic: Shortcode Builder
Result:There was confusion about what Ravinder's PR actually did. It turns out it does exactly what Matt was hoping for and this should resolve the user issue completely.

@ravinderk ravinderk closed this Jun 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.