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

email missing in ACCEPTURL in invite message body #157

Open
gyrus opened this issue Jun 27, 2018 · 7 comments
Open

email missing in ACCEPTURL in invite message body #157

gyrus opened this issue Jun 27, 2018 · 7 comments

Comments

@gyrus
Copy link

gyrus commented Jun 27, 2018

This is currently the email copy being generated on our site. Please note the email parameter - missing in the body, included in the footer:

You have been invited by XYZ to join the XYZ website

To accept this invitation, please visit https://[xyz]/?iaaction=accept-invitation&email

================
To opt out of future invitations to this site, please visit https://[xyz]/?iaaction=opt-out&email=[invitee's email]

The settings under Settings > Invite Anyone are:

Main text of email invitation message

You have been invited by %%INVITERNAME%% to join the XYZ website

To accept this invitation, please visit %%ACCEPTURL%%

Footer text of email invitation message (not editable by users)

To opt out of future invitations to this site, please visit %%OPTOUTURL%%

Looking through the code, it seems obvious why the email is missing from the body. All wildcard replacements are done here:

https://github.com/boonebgorges/invite-anyone/blob/1.3.20/by-email/by-email.php#L997

But the only call to this function which includes the 2nd $email parameter is this:

https://github.com/boonebgorges/invite-anyone/blob/1.3.20/by-email/by-email.php#L1266

This is for the footer - and in our received email you can see that %%OPTOUTURL%% is correctly including the email. However, as far as I can tell the wildcard replacements for the message body are being done here:

https://github.com/boonebgorges/invite-anyone/blob/1.3.20/by-email/by-email.php#L970

And the email isn't passed.

This seems like such a basic bug, I must be missing something!?

@boonebgorges
Copy link
Owner

Hi @gyrus - No, I don't think you're missing something. I guess the bug has never surfaced because I've never heard a report of someone using an email-sensitive replacement token (like %%ACCEPTURL%%) in the message body. (This, in turn, is probably because the email-less %%ACCEPTURL%% would be visible to users in the Message textarea when inviting members to the site.)

In any case, it certainly seems fine to pass the $email parameter when we've got it.

@boonebgorges
Copy link
Owner

Looking more closely at the logic, it will take some care to make this change without breaking other things. When processing an invitation-send, the message content is set here https://github.com/boonebgorges/invite-anyone/blob/1.3.20/by-email/by-email.php#L1166, before we have begun to loop through emails. That logic would have to be moved into this loop https://github.com/boonebgorges/invite-anyone/blob/1.3.20/by-email/by-email.php#L1260. But this would result in inconsistent results for $returned_data, which is what the function returns when there are errors during the send process and the invitation screen needs to be rerendered. This suggests that the "message" variable would need to be built twice: once outside the loop (the "generic" version) and once within (the version that's specific to the recipient). Ideally, this will be done without running the text through invite_anyone_wildcard_replace() more than once.

For the purposes of the "generic" version, it might be helpful to pass a dummy email address into invite_anyone_wildcard_replace(), so that the text that the user sees in the Message field contains URLs that make sense: ...&[email protected], or something like that.

With those caveats, a pull request would be quite welcome. Barring that, it's something I can try to look at myself when I've got a chance.

@gyrus
Copy link
Author

gyrus commented Jun 27, 2018

@boonebgorges Thanks. So the quick fix is to only use %%ACCEPTURL%% in the footer?

You might have to contextualise things a bit for me to follow things here. When you say the issue is that:

the email-less %%ACCEPTURL%% would be visible to users in the Message textarea when inviting members to the site

Visible to which users? In which textarea, where?

I don't quite follow the need for a version of the message outside the loop. The settings screen shows 'Replacement patterns for email text fields', followed by three fields - it's implicit that any wildcard could be used in any of those fields. %%ACCEPTURL%% and %%OPTOUTURL%% would both include the invitee's email address, so all three fields need to be processed for wildcards for every email being sent out.

@gyrus
Copy link
Author

gyrus commented Jun 27, 2018

@boonebgorges I'd be happy to submit a pull request if you're busy, but I'm wary as I don't understand the logic here. Maybe it's best for us to hack the plugin for ourselves until you've got time to do a fix?

@boonebgorges
Copy link
Owner

Yes, the quick fix is to use %%ACCEPTURL%% only in the footer.

Here are some screenshots to show what I mean. If you put %%ACCEPTURL%% in the message field, this is what users see when they are inviting:

screenshot_2018-06-27_09-10-06

Note the malformed URL (the bad email parameter you see in the text of the email you report above).

Now, if you submit the form but there are errors - say, you invite to a bad address - you're returned to the first screen. For convenience, the form is pre-filled with the values you provided when you originally filled out the form. So, you'd need to save the text as it was submitted separately from the text with the email addresses put in place.

The reason this doesn't matter so much for you is that you probably have disabled the ability for users to customize the message content. But this makes it even more complicated. Currently, the logic of wildcard replacement involves swapping out strings like %%ACCEPTURL%%. But you'll notice in the screenshot above that %%ACCEPTURL%% has been removed (swapped out with an empty string), so the replacement won't work as written. So the text displayed in the Message field might have to contain the literal %%ACCEPTURL%% string - that is, it should not be run through invite_anyone_wildcard_replace() - so that the replacement can happen properly at the time of sending.

It's all a bit complex and I may not be explaining it well, so sorry for the confusion!

@gyrus
Copy link
Author

gyrus commented Jun 27, 2018

@boonebgorges Yes, sounds like this relates to functionality we don't use.

It sounds quite complex, but the simple bit seems to be: better to have the 'malformed URL' there in the customised version textarea for those who use that feature, than to have it in the actual email being sent out for everyone?

Sounds like our best bet is to:

  1. Put the %%ACCEPTURL%% in the footer as a very quick fix.
  2. When we get time, hack the plugin on our site so it works for how we use it.
  3. You can fix the issue in a way that supports all use cases as and when.

@boonebgorges
Copy link
Owner

better to have the 'malformed URL' there in the customised version textarea for those who use that feature, than to have it in the actual email being sent out for everyone?

The former is responsible for the latter. The emails that go out to users are built from the text that appears in this textarea. It's done this way to allow users to customize the message in the outgoing email. So it has to be fixed holistically.

However, if your site will not be allowing customization of this message, you can probably hack around some of this by hardcoding some values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants