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

Moved the update_dates() call to update them after updating the subscription status #202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bernattorras
Copy link

Description

When you're importing expired subscriptions with their "End date", this date is automatically replaced by the current date (the importation time).

Steps to replicate

  1. Make sure that you have an expired subscription on your site (if not, create one and change its "end date" [_schedule_end] from the database to have an old date)
  2. Export this subscription
  3. Delete the subscription
  4. Import it and check if its 'End date' has the same date as the original subscription

Expected results

After importing the subscription, its "End date" should be the same date that was specified in the CSV.

Result

The "End date" of the imported subscription is replaced with the current date.

Origin of the issue

I see that the plugin updates the dates of the subscription before updating its status. In this case, when the status of a subscription is updated to "Expired", it overwrites the "End date" with the current date.

Note

I suggest to move the update_dates() call after the update_status() to make sure that the imported dates are preserved. I'm not sure though if removing this call from the previous location affects any specific cases (at first glance, this change doesn't seem problematic, as both methods will be called anyway, and the doesn't seem to be any specific reason to update the subscription dates before).

Fixes #201

@menakas menakas self-requested a review February 21, 2019 12:57
Copy link
Contributor

@menakas menakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bernattorras
Thanks for the prompt fix.

Yes, the call to update_status to set the status to expired sets the end_date to the current time.
While calling update_dates after the changing status will set the end date properly, I am not certain if it will affect other case.

So, probably, it is better to take a cautious approach to update only the end date after the status is updated. i.e,

  1. Leave the update_dates call where it was earlier.
  2. Call update_dates for just the end_date after the update_status call.

What do you think?

@bernattorras
Copy link
Author

Thanks for the suggestion @menakas.

Yes, I also thought about this approach, but I thought that it would be a bit redundant to call the same function twice. In any case, if you think that there are cases that will be affected, I agree that it will be safer to call it twice! :)

@leightona
Copy link

@bernattorras I tested the calling twice method on our magazine subs import and it worked perfectly setting the End Date to the imported value rather than the current time at import.
Thanks for your help with ironing this out, it saved us a lot of time and now allows us to accurately target win-back campaigns for expired subscribers.

@renansavidan
Copy link

renansavidan commented Aug 8, 2019

Hi same issue here - did you fix this ?
my end_date gets the current value.
I am trying to use _schedule_end from wp_postmeta now because it is basically not working for me.
If you know why, I am interested.

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

Successfully merging this pull request may close these issues.

Can't import the "End date" of an expired subscription
4 participants