Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

Fixes presignup'ing#6078

Merged
itsjoekent merged 4 commits intoDoSomethingArchive:devfrom
itsjoekent-archive:6070
Jan 25, 2016
Merged

Fixes presignup'ing#6078
itsjoekent merged 4 commits intoDoSomethingArchive:devfrom
itsjoekent-archive:6070

Conversation

@itsjoekent
Copy link
Contributor

What's this PR do?

Uses a different query for presignups

How should this be manually tested?

Does going to a closed campaign page / clicking presignup button work?

Any background context you want to provide?

starting with this comment (and everything after I wrote) #6070 (comment)

What are the relevant tickets?

Fixes #6070

Copy link
Contributor

Choose a reason for hiding this comment

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

First, you need to check if (!is_null($result)), as execute method may return NULL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you don't need to pass 0 as an argument, it's default value.

@itsjoekent
Copy link
Contributor Author

@sergii-tkachenko fixed!

@itsjoekent
Copy link
Contributor Author

@sergii-tkachenko alright, now its good haha

@sergiitk
Copy link
Contributor

Also, @return statement in the docblock should say int|false, not mixed.

@itsjoekent
Copy link
Contributor Author

@sergii-tkachenko hows that?

@sergiitk
Copy link
Contributor

👍 :shipit:

itsjoekent pushed a commit that referenced this pull request Jan 25, 2016
@itsjoekent itsjoekent merged commit d6c66f6 into DoSomethingArchive:dev Jan 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Closed campaign notify me breaks pages

2 participants