CampaignMonitorSignupPage_Controller   F
last analyzed

Complexity

Total Complexity 61

Size/Duplication

Total Lines 525
Duplicated Lines 5.52 %

Coupling/Cohesion

Components 5
Dependencies 19

Importance

Changes 0
Metric Value
wmc 61
lcom 5
cbo 19
dl 29
loc 525
rs 2.4812
c 0
b 0
f 0

24 Methods

Rating   Name   Duplication   Size   Complexity  
A init() 0 5 1
C SignupForm() 0 49 8
C subscribe() 0 74 11
A unsubscribe() 11 11 2
A confirm() 0 8 1
A thankyou() 0 8 1
A sadtoseeyougo() 0 8 1
B preloademail() 0 23 6
A viewcampaign() 9 9 2
A viewcampaigntextonly() 9 9 2
B previewcampaign() 0 11 5
A previewcampaigntextonly() 0 9 2
A Email() 0 4 1
A IsThankYou() 0 4 1
A IsConfirm() 0 4 1
A IsUnsubscribe() 0 4 1
A HasCampaign() 0 4 2
A Campaign() 0 4 1
A PreviousCampaignMonitorCampaigns() 0 14 2
B stats() 0 32 3
A CampaignStats() 0 20 3
A resetsignup() 0 5 1
A resetoldcampaigns() 0 10 2
A JSHackForPreSections() 0 21 1

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like CampaignMonitorSignupPage_Controller often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use CampaignMonitorSignupPage_Controller, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
/**
4
 * Page for Signing Up to Campaign Monitor List
5
 *
6
 * Each page relates to one CM list.
7
 *
8
 * @author nicolaas [at] sunnysideup.co.nz
9
 */
10
class CampaignMonitorSignupPage extends Page
0 ignored issues
show
Coding Style Compatibility introduced by
PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions.

You can fix this by adding a namespace to your class:

namespace YourVendor;

class YourClass { }

When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.

Loading history...
11
{
12
13
    /**
14
     * standard SS variable
15
     * @Var String
16
     */
17
    private static $singular_name = "Newsletter sign-up page";
0 ignored issues
show
Unused Code introduced by
The property $singular_name is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
18
    public function i18n_singular_name()
19
    {
20
        return _t("AccountPage.NEWSLETTER_PAGE", "Newsletter sign-up page");
21
    }
22
23
    /**
24
     * standard SS variable
25
     * @Var String
26
     */
27
    private static $plural_name = "Newsletter sign-up pages";
0 ignored issues
show
Unused Code introduced by
The property $plural_name is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
28
    public function i18n_plural_name()
29
    {
30
        return _t("AccountPage.NEWSLETTER_PAGE", "Newsletter sign-up pages");
31
    }
32
33
    /**
34
     *
35
     * @inherited
36
     */
37
    private static $icon = "campaignmonitor/images/treeicons/CampaignMonitorSignupPage";
0 ignored issues
show
Unused Code introduced by
The property $icon is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
38
39
    /**
40
     *
41
     * @inherited
42
     */
43
    private static $db = array(
0 ignored issues
show
Unused Code introduced by
The property $db is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
44
        'ListID' => 'Varchar(32)',
45
46
        'ConfirmTitle' => 'Varchar(255)',
47
        'ConfirmMenuTitle' => 'Varchar(255)',
48
        'ConfirmMessage' => 'HTMLText',
49
50
        'ThankYouTitle' => 'Varchar(255)',
51
        'ThankYouMenuTitle' => 'Varchar(255)',
52
        'ThankYouMessage' => 'HTMLText',
53
54
        'SadToSeeYouGoTitle' => 'Varchar(255)',
55
        'SadToSeeYouGoMenuTitle' => 'Varchar(255)',
56
        'SadToSeeYouGoMessage' => 'HTMLText',
57
58
        'SignUpHeader' => 'Varchar(100)',
59
        'SignUpIntro' => 'HTMLText',
60
        'SignUpButtonLabel' => 'Varchar(20)',
61
62
        'ShowOldNewsletters' => 'Boolean',
63
        'ShowAllNewsletterForSigningUp' => 'Boolean',
64
65
    );
66
67
    /**
68
     *
69
     * @inherited
70
     */
71
    private static $has_one = array(
0 ignored issues
show
Unused Code introduced by
The property $has_one is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
72
        "Group" => "Group"
73
    );
74
75
    /**
76
     *
77
     * @inherited
78
     */
79
    private static $has_many = array(
0 ignored issues
show
Unused Code introduced by
The property $has_many is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
80
        "CampaignMonitorSegments" => "CampaignMonitorSegment",
81
        "CampaignMonitorCustomFields" => "CampaignMonitorCustomField"
82
    );
83
84
    /**
85
     *
86
     * @inherited
87
     */
88
    private static $belongs_many_many = array(
0 ignored issues
show
Unused Code introduced by
The property $belongs_many_many is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
89
        "CampaignMonitorCampaigns" => "CampaignMonitorCampaign"
90
    );
91
92
    /**
93
     *
94
     * @inherited
95
     */
96
    private static $description = "Page to suscribe and review newsletter list(s)";
0 ignored issues
show
Unused Code introduced by
The property $description is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
97
98
    /**
99
     *
100
     * @var CampaignMonitorAPIConnector | Null
101
     */
102
    protected static $_api = null;
103
104
105
    /**
106
     * Campaign monitor pages that are ready to receive "sign-ups"
107
     * @return ArrayList
108
     */
109
    public static function get_ready_ones()
110
    {
111
        $listPages = CampaignMonitorSignupPage::get();
112
        $array = array(0 => 0);
113
        foreach ($listPages as $listPage) {
114
            if ($listPage->ReadyToReceiveSubscribtions()) {
115
                $array[$listPage->ID] = $listPage->ID;
116
            }
117
        }
118
        return CampaignMonitorSignupPage::get()->filter(array("ID" => $array));
119
    }
120
121
    /**
122
     *
123
     * @inherited
124
     */
125
    public function getCMSFields()
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
126
    {
127
        $fields = parent::getCMSFields();
128
        if ($this->GroupID) {
129
            $groupLink = '<h2><a href="/admin/security/EditForm/field/Groups/item/'.$this->GroupID.'/edit">open related security group</a></h2>';
130
        } else {
131
            $groupLink = '<p>No Group has been selected yet.</p>';
132
        }
133
        $testControllerLink = Injector::inst()->get("CampaignMonitorAPIConnector_TestController")->Link();
134
        $campaignExample = CampaignMonitorCampaign::get()->Last();
135
        $campaignExampleLink = $this->Link();
136
        if ($campaignExample) {
137
            $campaignExampleLink = $this->Link("viewcampaign/".$campaignExample->CampaignID);
138
        }
139
        if ($this->ID) {
140
            $config = GridFieldConfig_RelationEditor::create();
141
            $campaignField = new GridField('CampaignList', 'Campaigns', $this->CampaignMonitorCampaigns(), $config);
142
        } else {
143
            $campaignField = new HiddenField("CampaignList");
144
        }
145
        $gridFieldTemplatesAvailable = new GridField('TemplatesAvailable', 'Templates Available', CampaignMonitorCampaignStyle::get(), GridFieldConfig_RecordEditor::create());
146
        $gridFieldTemplatesAvailable->setDescription("Ask your developer on how to add more templates");
147
        $fields->addFieldToTab(
148
            'Root.AlternativeContent',
149
            new TabSet(
150
                "AlternativeContentSubHeader",
151
                    new Tab(
152
                        'Confirm',
153
                    new TextField('ConfirmTitle', 'Title'),
154
                    new TextField('ConfirmMenuTitle', 'Menu Title'),
155
                    new HtmlEditorField('ConfirmMessage', 'Message (e.g. thank you for confirming)')
156
                ),
157
                new Tab(
158
                    'ThankYou',
159
                    new TextField('ThankYouTitle', 'Title'),
160
                    new TextField('ThankYouMenuTitle', 'Menu Title'),
161
                    new HtmlEditorField('ThankYouMessage', 'Thank you message after submitting form')
162
                ),
163
                new Tab(
164
                    'SadToSeeYouGo',
165
                    new TextField('SadToSeeYouGoTitle', 'Title'),
166
                    new TextField('SadToSeeYouGoMenuTitle', 'Menu Title'),
167
                    new HtmlEditorField('SadToSeeYouGoMessage', 'Sad to see you  go message after submitting form')
168
                )
169
            )
170
        );
171
        $fields->addFieldToTab(
172
            'Root.Newsletters',
173
            new TabSet(
174
                'Options',
175
                new Tab(
176
                    'MainSettings',
177
                    new LiteralField('CreateNewCampaign', '<p>To create a new mail out go to <a href="'. Config::inst()->get("CampaignMonitorAPIConnector", "campaign_monitor_url") .'">Campaign Monitor</a> site.</p>'),
178
                    new LiteralField('ListIDExplanation', '<p>Each sign-up page needs to be associated with a campaign monitor subscription list.</p>'),
179
                    new DropdownField('ListID', 'Related List from Campaign Monitor (*)', array(0 => "-- please select --") + $this->makeDropdownListFromLists()),
180
                    new CheckboxField('ShowAllNewsletterForSigningUp', 'Allow users to sign up to all lists')
181
                ),
182
                new Tab(
183
                    'StartForm',
184
                    new LiteralField('StartFormExplanation', 'A start form is a form where people are just required to enter their email address and nothing else.  After completion they go through to another page (the actual CampaignMonitorSignUpPage) to complete all the details.'),
185
                    new TextField('SignUpHeader', 'Sign up header (e.g. sign up now)'),
186
                    new HtmlEditorField('SignUpIntro', 'Sign up form intro (e.g. sign up for our monthly newsletter ...'),
187
                    new TextField('SignUpButtonLabel', 'Sign up button label for start form (e.g. register now)')
188
                ),
189
                new Tab(
190
                    'Newsletters',
191
                    new CheckboxField('ShowOldNewsletters', 'Show old newsletters? Set to "NO" to remove all old newsletters links to this page. Set to "YES" to retrieve all old newsletters.'),
192
                    new LiteralField('CampaignExplanation', '<h3>Unfortunately, newsletter lists are not automatically linked to individual newsletters, you can link them here...</h3>'),
193
                    new CheckboxSetField('CampaignMonitorCampaigns', 'Newsletters shown', CampaignMonitorCampaign::get()->filter("HasBeenSent", 1)->limit(10000)->map()->toArray()),
0 ignored issues
show
Bug introduced by
The method toArray cannot be called on \CampaignMonitorCampaign...1)->limit(10000)->map() (of type array).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
194
                    $campaignField,
195
                    $gridFieldTemplatesAvailable
196
                ),
197
                new Tab(
198
                    'Advanced',
199
                    new LiteralField('MyControllerTest', '<h3><a href="'.$testControllerLink.'">Test Connections</a></h3>'),
200
                    new LiteralField('MyStats', '<h3><a href="'.$this->Link("stats").'">Stats and Debug information</a></h3>'),
201
                    new LiteralField('MyCampaignReset', '<h3><a href="'.$this->Link("resetoldcampaigns").'">Delete All Campaigns from Website</a></h3>'),
202
                    new LiteralField('MyCampaignInfo', '<h3>You can also view individual campaigns - here is <a href="'.$campaignExampleLink.'">an example</a></h3>'),
203
                    $gridField = new GridField('Segments', 'Segments', $this->CampaignMonitorSegments(), GridFieldConfig_RecordViewer::create()),
204
                    $gridField = new GridField('CustomFields', 'Custom Fields', $this->CampaignMonitorCustomFields(), GridFieldConfig_RecordViewer::create()),
205
                    new LiteralField('GroupLink', $groupLink)
206
                )
207
            )
208
        );
209
        if (!Config::inst()->get("CampaignMonitorWrapper", "campaign_monitor_url")) {
0 ignored issues
show
Unused Code introduced by
This if statement is empty and can be removed.

This check looks for the bodies of if statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.

These if bodies can be removed. If you have an empty if but statements in the else branch, consider inverting the condition.

if (rand(1, 6) > 3) {
//print "Check failed";
} else {
    print "Check succeeded";
}

could be turned into

if (rand(1, 6) <= 3) {
    print "Check succeeded";
}

This is much more concise to read.

Loading history...
210
            //$fields->removeFieldFromTab("Root.Newsletters.Options", "CreateNewCampaign");
0 ignored issues
show
Unused Code Comprehensibility introduced by
80% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
211
        }
212
        return $fields;
213
    }
214
215
    /**
216
     *
217
     * @return CampaignMonitorAPIConnector
218
     */
219
    public function getAPI()
220
    {
221
        if (!self::$_api) {
222
            self::$_api = CampaignMonitorAPIConnector::create();
223
            self::$_api->init();
224
        }
225
        return self::$_api;
226
    }
227
228
    /**
229
     *
230
     * @var Null | Array
231
     */
232
    private static $drop_down_list = array();
233
234
    /**
235
     * returns available list for client
236
     * @return array
237
     */
238
    protected function makeDropdownListFromLists()
239
    {
240
        if (!isset(self::$drop_down_list[$this->ID])) {
241
            $array = array();
242
            $api = $this->getAPI();
243
            $lists = $api->getLists();
244
            if (is_array($lists) && count($lists)) {
245
                foreach ($lists as $list) {
246
                    $array[$list->ListID] = $list->Name;
247
                }
248
            }
249
            //remove subscription list IDs from other pages
250
            $subscribePages = CampaignMonitorSignupPage::get()
251
                ->exclude("ID", $this->ID);
252
            foreach ($subscribePages as $page) {
253
                if (isset($array[$page->ListID])) {
254
                    unset($array[$page->ListID]);
255
                }
256
            }
257
            self::$drop_down_list[$this->ID] = $array;
258
        }
259
        return self::$drop_down_list[$this->ID];
260
    }
261
262
    /**
263
     * you can add this function to other pages to have a form
264
     * that starts the basic after which the client needs to complete the rest.
265
     *
266
     * Or does a basic sign up if ajax submitted.
267
     *
268
     * @param Controller $controller
269
     * @param String $formName
270
     *
271
     * @return Form
272
     */
273
    public function CampaignMonitorStartForm(Controller $controller, $formName = "CampaignMonitorStarterForm")
274
    {
275
        if ($email = Session::get("CampaignMonitorStartForm_AjaxResult_".$this->ID)) {
276
            return $this->renderWith("CampaignMonitorStartForm_AjaxResult", array("Email" => $email));
277
        } else {
278
            Requirements::javascript(THIRDPARTY_DIR . '/jquery/jquery.js');
279
            //Requirements::javascript(THIRDPARTY_DIR . '/jquery-form/jquery.form.js');
0 ignored issues
show
Unused Code Comprehensibility introduced by
46% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
280
            Requirements::javascript(SS_CAMPAIGNMONITOR_DIR . '/javascript/CampaignMonitorStartForm.js');
281
            if (!$this->ReadyToReceiveSubscribtions()) {
282
                //user_error("You first need to setup a Campaign Monitor Page for this function to work.", E_USER_NOTICE);
0 ignored issues
show
Unused Code Comprehensibility introduced by
63% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
283
                return false;
284
            }
285
            $fields = new FieldList(new EmailField("CampaignMonitorEmail", _t("CAMPAIGNMONITORSIGNUPPAGE.EMAIL", "Email")));
286
            $actions = new FieldList(new FormAction("campaignmonitorstarterformstartaction", $this->SignUpButtonLabel));
287
            $form = new Form(
288
                $controller,
289
                $formName,
290
                $fields,
291
                $actions
292
            );
293
            $form->setFormAction($this->Link("preloademail"));
294
            return $form;
295
        }
296
    }
297
298
    /**
299
     * adds a subcriber to the list without worrying about making it a user ...
300
     *
301
     * @param String $email
302
     *
303
     * @returns
304
     */
305
    public function addSubscriber($email)
306
    {
307
        if ($this->ReadyToReceiveSubscribtions()) {
308
            $listID = $this->ListID;
309
            $email = Convert::raw2sql($email);
310
            $member = Member::get()->filter(array("Email" => $email))->first();
311
            if ($member && $member->exists()) {
0 ignored issues
show
Unused Code introduced by
This if statement is empty and can be removed.

This check looks for the bodies of if statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.

These if bodies can be removed. If you have an empty if but statements in the else branch, consider inverting the condition.

if (rand(1, 6) > 3) {
//print "Check failed";
} else {
    print "Check succeeded";
}

could be turned into

if (rand(1, 6) <= 3) {
    print "Check succeeded";
}

This is much more concise to read.

Loading history...
312
                //do nothing
313
            } else {
314
                $member = new Member();
315
                $member->Email = $email;
316
                $member->SetPassword = true;
0 ignored issues
show
Bug introduced by
The property SetPassword does not seem to exist. Did you mean Password?

An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name.

If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading.

Loading history...
317
                $member->Password = Member::create_new_password();
0 ignored issues
show
Deprecated Code introduced by
The method Member::create_new_password() has been deprecated with message: 3.6.0..4.0.0

This method has been deprecated. The supplier of the class has supplied an explanatory message.

The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead.

Loading history...
318
                $member->write();
319
            }
320
            if ($group = $this->Group()) {
321
                $group->Members()->add($member);
322
            }
323
            $api = $this->getAPI();
324
            $result = $api->addSubscriber($listID, $member);
0 ignored issues
show
Compatibility introduced by
$member of type object<DataObject> is not a sub-type of object<Member>. It seems like you assume a child class of the class DataObject to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
325
            if ($result == $email) {
326
                return null;
327
            }
328
            return "ERROR: could not subscribe";
329
        }
330
        return "ERROR: not ready";
331
    }
332
333
    /**
334
     * name of the list connected to.
335
     * @return String
336
     */
337
    public function getListTitle()
338
    {
339
        if ($this->ListID) {
340
            $a = $this->makeDropdownListFromLists();
341
            if (isset($a[$this->ListID])) {
342
                return $a[$this->ListID];
343
            }
344
        }
345
        return "";
346
    }
347
348
    /**
349
     * tells us if the page is ready to receive subscriptions
350
     * @return Boolean
351
     */
352
    public function ReadyToReceiveSubscribtions()
353
    {
354
        return $this->ListID && $this->GroupID;
355
    }
356
357
    /**
358
     * check list and group IDs
359
     *
360
     */
361
    public function onBeforeWrite()
362
    {
363
        parent::onBeforeWrite();
364
        //check list
365
        if (!$this->getListTitle()) {
366
            $this->ListID = 0;
367
        }
368
        $gp = null;
369
        //check group
370
        if ($this->GroupID) {
371
            $gp = $this->Group();
372
            if (!$gp || !$gp->exists()) {
373
                $this->GroupID = 0;
374
            }
375
        }
376
377
        //add group
378
        if ($this->ListID) {
379
            if (!$this->GroupID) {
380
                $gp = new Group();
381
                $this->GroupID = $gp->ID;
382
                $gp->write();
383
            }
384
            $title = _t("CampaignMonitor.NEWSLETTER", "NEWSLETTER");
385
            if ($myListName = $this->getListTitle()) {
386
                $title .= ": ".$myListName;
387
            }
388
            $gp->Title = (string)$title;
389
            $gp->write();
390
        }
391
        if ($gp) {
392
            $this->GroupID = $gp->ID;
393
        }
394
    }
395
396
    /**
397
     * add old campaings or remove them
398
     * depending on the setting
399
     *
400
     * add / remove segments ...
401
     *
402
     */
403
    public function onAfterWrite()
404
    {
405
        parent::onAfterWrite();
406
        if ($this->ShowOldNewsletters) {
407
            $this->AddOldCampaigns();
408
        } else {
409
            $this->CampaignMonitorCampaigns()->filter(array("HasBeenSent" => 1))->removeAll();
410
        }
411
        //add segments
412
        $segmentsAdded = array();
413
        $segments = $this->api->getSegments($this->ListID);
414
        if ($segments && is_array($segments) && count($segments)) {
415
            foreach ($segments as $segment) {
416
                $segmentsAdded[$segment->SegmentID] = $segment->SegmentID;
417
                $filterArray = array("SegmentID" => $segment->SegmentID, "ListID" => $this->ListID, "CampaignMonitorSignupPageID" => $this->ID);
418
                $obj = CampaignMonitorSegment::get()->filter($filterArray)->first();
419
                if (!$obj) {
420
                    $obj = CampaignMonitorSegment::create($filterArray);
421
                }
422
                $obj->Title = $segment->Title;
423
                $obj->write();
0 ignored issues
show
Bug introduced by
The method write does only exist in DataObject, but not in CampaignMonitorSignupPage.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
424
            }
425
        }
426
        $unwantedSegments = CampaignMonitorSegment::get()->filter(array("ListID" => $this->ListID, "CampaignMonitorSignupPageID" => $this->ID))
427
            ->exclude(array("SegmentID" => $segmentsAdded));
428
        foreach ($unwantedSegments as $unwantedSegment) {
429
            $unwantedSegment->delete();
430
        }
431
        //add custom fields
432
        $customCustomFieldsAdded = array();
433
        $customCustomFields = $this->api->getListCustomFields($this->ListID);
434
        if ($customCustomFields && is_array($customCustomFields) && count($customCustomFields)) {
435
            foreach ($customCustomFields as $customCustomField) {
436
                $obj = CampaignMonitorCustomField::create_from_campaign_monitor_object($customCustomField, $this->ListID);
437
                $customCustomFieldsAdded[$obj->Code] = $obj->Code;
0 ignored issues
show
Documentation introduced by
The property Code does not exist on object<CampaignMonitorCustomField>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
Documentation introduced by
The property Code does not exist on object<CampaignMonitorCustomField>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
438
            }
439
        }
440
        $unwantedCustomFields = CampaignMonitorCustomField::get()->filter(array("ListID" => $this->ListID, "CampaignMonitorSignupPageID" => $this->ID))
441
            ->exclude(array("Code" => $customCustomFieldsAdded));
442
        foreach ($unwantedCustomFields as $unwantedCustomField) {
443
            $unwantedCustomField->delete();
444
        }
445
    }
446
447
    public function AddOldCampaigns()
448
    {
449
        $task = CampaignMonitorAddOldCampaigns::create();
450
        $task->setVerbose(false);
451
        $task->run(null);
452
    }
453
454
    public function requireDefaultRecords()
455
    {
456
        parent::requireDefaultRecords();
457
        $update = array();
458
        $page = CampaignMonitorSignupPage::get()->First();
459
460
        if ($page) {
461
            if (!$page->SignUpHeader) {
462
                $page->SignUpHeader = 'Sign Up Now';
463
                $update[]= "created default entry for SignUpHeader";
464
            }
465
            if (strlen($page->SignUpIntro) < strlen("<p> </p>")) {
466
                $page->SignUpIntro = '<p>Enter your email to sign up for our newsletter</p>';
467
                $update[]= "created default entry for SignUpIntro";
468
            }
469
            if (!$page->SignUpButtonLabel) {
470
                $page->SignUpButtonLabel = 'Register Now';
471
                $update[]= "created default entry for SignUpButtonLabel";
472
            }
473
            if (count($update)) {
474
                $page->writeToStage('Stage');
475
                $page->publish('Stage', 'Live');
476
                DB::alteration_message($page->ClassName." created/updated: ".implode(" --- ", $update), 'created');
477
            }
478
        }
479
    }
480
}
481
482
class CampaignMonitorSignupPage_Controller extends Page_Controller
0 ignored issues
show
Coding Style Compatibility introduced by
PSR1 recommends that each class should be in its own file to aid autoloaders.

Having each class in a dedicated file usually plays nice with PSR autoloaders and is therefore a well established practice. If you use other autoloaders, you might not want to follow this rule.

Loading history...
Coding Style Compatibility introduced by
PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions.

You can fix this by adding a namespace to your class:

namespace YourVendor;

class YourClass { }

When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.

Loading history...
483
{
484
485
    /**
486
     * retains email for processing
487
     * @var boolean
488
     */
489
    protected $isThankYou = false;
490
491
    /**
492
     * retains email for processing
493
     * @var boolean
494
     */
495
    protected $isUnsubscribe = false;
496
497
    /**
498
     * retains email for processing
499
     * @var boolean
500
     */
501
    protected $isConfirm = false;
502
503
    /**
504
     * retains email for processing
505
     * @var String
506
     */
507
    protected $email = '';
508
509
    /**
510
     * holder for selected campaign
511
     *
512
     * @var Null | CampaignMonitorCampaign
513
     */
514
    protected $campaign = '';
515
516
    private static $allowed_actions = array(
0 ignored issues
show
Unused Code introduced by
The property $allowed_actions is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
517
        "SignupForm" => true,
518
        "subscribe" => true,
519
        "unsubscribe" => true,
520
        "thankyou" => true,
521
        "confirm" => true,
522
        "sadtoseeyougo" => true,
523
        "preloademail" => true,
524
        "viewcampaign" => true,
525
        "viewcampaigntextonly" => true,
526
        "previewcampaign" => true,
527
        "previewcampaigntextonly" => true,
528
        "stats" => true,
529
        "resetoldcampaigns" => true,
530
        "resetsignup" => true
531
    );
532
533
    public function init()
534
    {
535
        parent::init();
536
        Requirements::themedCSS("CampaignMonitorSignupPage", "campaignmonitor");
537
    }
538
539
    /**
540
     * creates a subscription form...
541
     * @return Form
542
     */
543
    public function SignupForm()
544
    {
545
        if ($this->ReadyToReceiveSubscribtions()) {
546
            // Create fields
547
            $member = Member::currentUser();
548
            $emailField = null;
549
            $emailRequired = true;
550
            if (!$member) {
551
                $member = new Member();
552
            } else {
553
                $this->email = $member->Email;
554
                if ($this->email) {
555
                    $emailRequired = false;
556
                    $emailField = new ReadonlyField('CampaignMonitorEmail', _t("CAMPAIGNMONITORSIGNUPPAGE.EMAIL", 'Email'), $this->email);
557
                }
558
            }
559
            if (!$emailField) {
560
                $emailField = new EmailField('CampaignMonitorEmail', _t("CAMPAIGNMONITORSIGNUPPAGE.EMAIL", 'Email'), $this->email);
561
            }
562
            if ($this->ShowAllNewsletterForSigningUp) {
563
                $signupField = $member->getCampaignMonitorSignupField(null, "SubscribeManyChoices");
564
            } else {
565
                $signupField = $member->getCampaignMonitorSignupField($this->ListID, "SubscribeChoice");
566
            }
567
            $fields = new FieldList(
568
                $emailField,
569
                $signupField
570
            );
571
            // Create action
572
            $actions = new FieldList(
573
                new FormAction('subscribe', _t("CAMPAIGNMONITORSIGNUPPAGE.UPDATE_SUBSCRIPTIONS", "Update Subscriptions"))
574
            );
575
            // Create Validators
576
            if ($emailRequired) {
577
                $validator = new RequiredFields('CampaignMonitorEmail');
578
            } else {
579
                $validator = new RequiredFields();
580
            }
581
            $form = new Form($this, 'SignupForm', $fields, $actions, $validator);
582
            if ($member->exists()) {
583
                $form->loadDataFrom($member);
584
            } else {
585
                $form->Fields()->fieldByName("CampaignMonitorEmail")->setValue($this->email);
586
            }
587
            return $form;
588
        } else {
589
            return _t("CampaignMonitorSignupPage.NOTREADY", "You can not suscribe to this newsletter at present.");
590
        }
591
    }
592
593
    /**
594
     * action subscription form
595
     * @param Array $array
0 ignored issues
show
Bug introduced by
There is no parameter named $array. Was it maybe removed?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function.

Consider the following example. The parameter $italy is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $island
 * @param array $italy
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was removed, but the annotation was not.

Loading history...
596
     * @param Form $form
597
     *
598
     * return redirect
599
     */
600
    public function subscribe($data, $form)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
601
    {
602
        if ($this->ReadyToReceiveSubscribtions()) {
603
            //true until proven otherwise.
604
            $newlyCreatedMember = false;
605
            $api = $this->getAPI();
0 ignored issues
show
Unused Code introduced by
$api is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
606
            $member = Member::currentUser();
607
608
            //subscribe or unsubscribe?
609
            if (isset($data["SubscribeManyChoices"])) {
610
                $isSubscribe = true;
611
            } else {
612
                $isSubscribe = isset($data["SubscribeChoice"]) && $data["SubscribeChoice"] == "Subscribe";
613
            }
614
615
            //no member logged in: if the member already exists then you can't sign up.
616
            if (!$member) {
617
                $memberAlreadyLoggedIn = false;
0 ignored issues
show
Unused Code introduced by
$memberAlreadyLoggedIn is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
618
                $filter = array("Email" => Convert::raw2sql($data["CampaignMonitorEmail"]));
619
                $existingMember = Member::get()->filter($filter)->First();
620
                //if($isSubscribe && $existingMember){
0 ignored issues
show
Unused Code Comprehensibility introduced by
78% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
621
                //$form->addErrorMessage('Email', _t("CAMPAIGNMONITORSIGNUPPAGE.EMAIL_EXISTS", "This email is already in use. Please log in for this email or try another email address."), 'warning');
0 ignored issues
show
Unused Code Comprehensibility introduced by
74% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
622
                //$this->redirectBack();
0 ignored issues
show
Unused Code Comprehensibility introduced by
84% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
623
                //return;
624
                //}
625
                $member = $existingMember;
626
                if (!$member) {
627
                    $newlyCreatedMember = true;
628
                    $member = Member::create($filter);
629
                }
630
            }
631
632
            //logged in: if the member already as someone else then you can't sign up.
633
            else {
634
                $memberAlreadyLoggedIn = true;
0 ignored issues
show
Unused Code introduced by
$memberAlreadyLoggedIn is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
635
                //$existingMember = Member::get()
0 ignored issues
show
Unused Code Comprehensibility introduced by
45% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
636
                //	->filter(array("Email" => Convert::raw2sql($data["CampaignMonitorEmail"])))
0 ignored issues
show
Unused Code Comprehensibility introduced by
72% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
637
                //	->exclude(array("ID" => $member->ID))
0 ignored issues
show
Unused Code Comprehensibility introduced by
67% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
638
                //	->First();
0 ignored issues
show
Unused Code Comprehensibility introduced by
67% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
639
                //if($isSubscribe && $existingMember) {
0 ignored issues
show
Unused Code Comprehensibility introduced by
70% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
640
                    //$form->addErrorMessage('CampaignMonitorEmail', _t("CAMPAIGNMONITORSIGNUPPAGE.EMAIL_EXISTS", "This email is already in use by someone else. Please log in for this email or try another email address."), 'warning');
0 ignored issues
show
Unused Code Comprehensibility introduced by
74% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
641
                    //$this->redirectBack();
0 ignored issues
show
Unused Code Comprehensibility introduced by
84% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
642
                    //return;
643
                //}
644
            }
645
646
            //are any choices being made
647
            if (!isset($data["SubscribeChoice"]) && !isset($data["SubscribeManyChoices"])) {
648
                $form->addErrorMessage('SubscribeChoice', _t("CAMPAIGNMONITORSIGNUPPAGE.NO_NAME", "Please choose your subscription."), 'warning');
649
                $this->redirectBack();
650
                return;
651
            }
652
653
            //if this is a new member then we save the member
654
            if ($isSubscribe) {
655
                if ($newlyCreatedMember) {
656
                    $form->saveInto($member);
657
                    $member->Email = Convert::raw2sql($data["CampaignMonitorEmail"]);
658
                    $member->SetPassword = true;
659
                    $member->Password = Member::create_new_password();
0 ignored issues
show
Deprecated Code introduced by
The method Member::create_new_password() has been deprecated with message: 3.6.0..4.0.0

This method has been deprecated. The supplier of the class has supplied an explanatory message.

The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead.

Loading history...
660
                    $member->write();
0 ignored issues
show
Bug introduced by
The method write does only exist in DataObject, but not in CampaignMonitorSignupPage_Controller.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
661
                    $member->logIn($keepMeLoggedIn = false);
662
                }
663
            }
664
            $outcome = $member->processCampaignMonitorSignupField($this->dataRecord, $data, $form);
665
            if ($outcome == "subscribe") {
666
                return $this->redirect($this->link("thankyou"));
667
            } else {
668
                return $this->redirect($this->link("sadtoseeyougo"));
669
            }
670
        } else {
671
            user_error("No list to subscribe to", E_USER_WARNING);
672
        }
673
    }
674
675
    /**
676
     * immediately unsubscribe if you are logged in.
677
     * @param HTTPRequest
678
     */
679 View Code Duplication
    public function unsubscribe($request)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
680
    {
681
        $member = Member::currentUser();
682
        if ($member) {
683
            $member->removeCampaignMonitorList($this->ListID);
684
            $this->Content = $member->Email." has been removed from this list: ".$this->getListTitle();
685
        } else {
686
            Security::permissionFailure($this, _t("CAMPAIGNMONITORSIGNUPPAGE.LOGINFIRST", "Please login first."));
687
        }
688
        return array();
689
    }
690
691
    /**
692
     * action
693
     * @param HTTPRequest
694
     */
695
    public function confirm($request)
696
    {
697
        $this->Title = $this->ConfirmTitle;
698
        $this->MenuTitle = $this->ConfirmMenuTitle;
699
        $this->Content = $this->ConfirmMessage;
700
        $this->isConfirm = true;
701
        return array();
702
    }
703
704
    /**
705
     * action
706
     * @param HTTPRequest
707
     */
708
    public function thankyou($request)
709
    {
710
        $this->Title = $this->ThankYouTitle;
711
        $this->MenuTitle = $this->ThankYouMenuTitle;
712
        $this->Content = $this->ThankYouMessage;
713
        $this->isThankYou = true;
714
        return array();
715
    }
716
717
    /**
718
     * action
719
     * @param HTTPRequest
720
     */
721
    public function sadtoseeyougo($request)
722
    {
723
        $this->Title = $this->SadToSeeYouGoTitle;
724
        $this->MenuTitle = $this->SadToSeeYouGoMenuTitle;
725
        $this->Content = $this->SadToSeeYouGoMessage;
726
        $this->isUnsubscribe = true;
727
        return array();
728
    }
729
730
    /**
731
     * action
732
     * @param HTTPRequest
733
     */
734
    public function preloademail(SS_HTTPRequest $request)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
735
    {
736
        $data = $request->requestVars();
737
        if (isset($data["CampaignMonitorEmail"])) {
738
            $email = Convert::raw2sql($data["CampaignMonitorEmail"]);
739
            if ($email) {
740
                $this->email = $email;
0 ignored issues
show
Documentation Bug introduced by
It seems like $email can also be of type array. However, the property $email is declared as type string. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
741
                if (Director::is_ajax()) {
742
                    if (!$this->addSubscriber($email)) {
743
                        Session::set("CampaignMonitorStartForm_AjaxResult_".$this->ID, $data["CampaignMonitorEmail"]);
744
                        return $this->renderWith("CampaignMonitorStartForm_AjaxResult");
745
                    } else {
746
                        return "ERROR";
747
                    }
748
                }
749
            }
750
        } else {
751
            if ($m = Member::currentUser()) {
752
                $this->email = $m->Email;
753
            }
754
        }
755
        return array();
756
    }
757
758
    /**
759
     *
760
     * action to show one campaign...
761
     */
762 View Code Duplication
    public function viewcampaign($request)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
763
    {
764
        $id = intval($request->param("ID"));
765
        $this->campaign = CampaignMonitorCampaign::get()->byID($id);
0 ignored issues
show
Documentation Bug introduced by
It seems like \CampaignMonitorCampaign::get()->byID($id) can also be of type object<DataObject>. However, the property $campaign is declared as type null. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
766
        if (!$this->campaign) {
767
            return $this->httpError(404, _t("CAMPAIGNMONITORSIGNUPPAGE.CAMPAIGN_NOT_FOUND", "Message not found."));
768
        }
769
        return array();
770
    }
771
772
    /**
773
     *
774
     * action to show one campaign TEXT ONLY...
775
     */
776 View Code Duplication
    public function viewcampaigntextonly($request)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
777
    {
778
        $id = intval($request->param("ID"));
779
        $this->campaign = CampaignMonitorCampaign::get()->byID($id);
0 ignored issues
show
Documentation Bug introduced by
It seems like \CampaignMonitorCampaign::get()->byID($id) can also be of type object<DataObject>. However, the property $campaign is declared as type null. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
780
        if (!$this->campaign) {
781
            return $this->httpError(404, _t("CAMPAIGNMONITORSIGNUPPAGE.CAMPAIGN_NOT_FOUND", "Message not found."));
782
        }
783
        return array();
784
    }
785
786
    /**
787
     *
788
     * action to preview one campaign...
789
     */
790
    public function previewcampaign($request)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
Coding Style introduced by
previewcampaign uses the super-global variable $_GET which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
791
    {
792
        $id = intval($request->param("ID"));
793
        $this->campaign = CampaignMonitorCampaign::get()->byID($id);
0 ignored issues
show
Documentation Bug introduced by
It seems like \CampaignMonitorCampaign::get()->byID($id) can also be of type object<DataObject>. However, the property $campaign is declared as type null. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
794
        if ($this->campaign) {
795
            if (isset($_GET["hash"]) && strlen($_GET["hash"]) == 7 &&  $_GET["hash"] == $this->campaign->Hash) {
796
                return HTTP::absoluteURLs($this->campaign->getNewsletterContent());
797
            }
798
        }
799
        return $this->httpError(404, _t("CAMPAIGNMONITORSIGNUPPAGE.CAMPAIGN_NOT_FOUND", "No preview available."));
800
    }
801
802
    /**
803
     *
804
     * action to preview one campaign TEXT ONLY...
805
     */
806
    public function previewcampaigntextonly($request)
0 ignored issues
show
Documentation introduced by
The return type could not be reliably inferred; please add a @return annotation.

Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a @return annotation as described here.

Loading history...
807
    {
808
        $id = intval($request->param("ID"));
809
        $this->campaign = CampaignMonitorCampaign::get()->byID($id);
0 ignored issues
show
Documentation Bug introduced by
It seems like \CampaignMonitorCampaign::get()->byID($id) can also be of type object<DataObject>. However, the property $campaign is declared as type null. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
810
        if ($this->campaign) {
811
            return HTTP::absoluteURLs(strip_tags($this->campaign->getNewsletterContent()));
812
        }
813
        return $this->httpError(404, _t("CAMPAIGNMONITORSIGNUPPAGE.CAMPAIGN_NOT_FOUND", "No preview available."));
814
    }
815
816
817
    /**
818
     *
819
     * @return String
820
     */
821
    public function Email()
822
    {
823
        return $this->email;
824
    }
825
826
    /**
827
     *
828
     * @return Boolean
829
     */
830
    public function IsThankYou()
831
    {
832
        return $this->isThankYou;
833
    }
834
835
    /**
836
     *
837
     * @return Boolean
838
     */
839
    public function IsConfirm()
840
    {
841
        return $this->isConfirm;
842
    }
843
844
    /**
845
     *
846
     * @return Boolean
847
     */
848
    public function IsUnsubscribe()
849
    {
850
        return $this->isUnsubscribe;
851
    }
852
853
    /**
854
     *
855
     * @return Boolean
856
     */
857
    public function HasCampaign()
858
    {
859
        return $this->campaign ? true : false;
860
    }
861
862
    /**
863
     *
864
     * @return Null | CampaignMonitorCampaign
865
     */
866
    public function Campaign()
867
    {
868
        return $this->campaign;
869
    }
870
871
872
    /**
873
     * same as $this->CampaignMonitorCampaigns()
874
     * but sorted correctly.
875
     *
876
     * @return DataSet of CampaignMonitorCampaigns
0 ignored issues
show
Documentation introduced by
Should the return type not be DataList|null?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
877
     */
878
    public function PreviousCampaignMonitorCampaigns()
879
    {
880
        if ($this->ShowOldNewsletters) {
881
            $campaigns = $this->CampaignMonitorCampaigns();
882
            return CampaignMonitorCampaign::get()
883
                ->filter(
884
                    array(
885
                        "ID" => $campaigns->map("ID", "ID")->toArray(),
886
                        "Hide" => 0,
887
                        "HasBeenSent" => 1
888
                    )
889
                );
890
        }
891
    }
892
893
    /**
894
     * action for admins only to see a whole bunch of
895
     * stats
896
     */
897
    public function stats()
898
    {
899
        if (Permission::check("Admin")) {
900
            //run tests here
901
            $api = $this->getAPI();
902
            $html = "<div id=\"CampaignMonitorStats\">";
903
            $html .= "<h1>Debug Response</h1>";
904
            $html .= "<h2>Main Client Stuff</h2>";
905
            $html .= "<h3>Link to confirm page</h3>".Director::absoluteUrl($this->Link("confirm"))."";
906
            $html .= "<h3>Link to thank-you-page</h3>".Director::absoluteUrl($this->Link("thankyou"))."";
907
            $html .= "<h3>Link to sad-to-see-you-go page</h3>".Director::absoluteUrl($this->Link("sadtoseeyougo"))."";
908
            $html .= "<h3><a href=\"#\">All Campaigns</a></a></h3><pre>".print_r($api->getCampaigns(), 1)."</pre>";
909
            $html .= "<h3><a href=\"#\">All Lists</a></h3><pre>".print_r($api->getLists(), 1)."</pre>";
910
            if ($this->ListID) {
911
                $html .= "<h2>List</h2";
912
                $html .= "<h3><a href=\"#\" id=\"MyListStatsAreHere\">List Stats</a></h3><pre>".print_r($api->getListStats($this->ListID), 1)."</pre>";
913
                $html .= "<h3><a href=\"#\">List Details</a></h3><pre>".print_r($api->getList($this->ListID), 1)."</pre>";
914
                $html .= "<h3><a href=\"#\">Active Subscribers (latest ones)</a></h3><pre>".print_r($api->getActiveSubscribers($this->ListID), 1)."</pre>";
915
                $html .= "<h3><a href=\"#\">Unconfirmed Subscribers (latest ones)</a></h3><pre>".print_r($api->getUnconfirmedSubscribers($this->ListID), 1)."</pre>";
916
                $html .= "<h3><a href=\"#\">Bounced Subscribers (latest ones)</a></h3><pre>".print_r($api->getBouncedSubscribers($this->ListID), 1)."</pre>";
917
                $html .= "<h3><a href=\"#\">Unsubscribed Subscribers (latest ones)</a></h3><pre>".print_r($api->getUnsubscribedSubscribers($this->ListID), 1)."</pre>";
918
            } else {
919
                $html .= "<h2 style=\"color: red;\">ERROR: No Lists selected</h2";
920
            }
921
            Requirements::customScript($this->JSHackForPreSections(), "CampaignMonitorStats");
922
            $html .= "</div>";
923
            $this->Content = $html;
924
        } else {
925
            Security::permissionFailure($this, _t("CAMPAIGNMONITORSIGNUPPAGE.TESTFAILURE", "This function is only available for administrators"), 1);
0 ignored issues
show
Unused Code introduced by
The call to Security::permissionFailure() has too many arguments starting with 1.

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
926
        }
927
        return array();
928
    }
929
930
    /**
931
     * returns a bunch of stats about a campaign
932
     * IF the user is an admin AND a campaign is selected
933
     * @return String (html) | false
0 ignored issues
show
Documentation introduced by
Should the return type not be null|false?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
934
     */
935
    public function CampaignStats()
936
    {
937
        if (Permission::check("CMS_ACCESS_CMSMain")) {
938
            if ($this->campaign) {
939
                //run tests here
940
                $api = $this->getAPI();
941
                $html = "<div id=\"CampaignMonitorStats\">";
942
                $html .= "<h2>Campaign Stats</h2>";
943
                $html .= "<h3><a href=\"#\">Campaign: ".$this->campaign->Subject."</a></h3>";
944
                $html .= "<h3><a href=\"#\">Summary</a></h3><pre>".print_r($api->getSummary($this->campaign->CampaignID), 1)."</pre>";
945
                $html .= "<h3><a href=\"#\">Email Client Usage</a></h3><pre>".print_r($api->getEmailClientUsage($this->campaign->CampaignID), 1)."</pre>";
946
                $html .= "<h3><a href=\"#\">Unsubscribes</a></h3><pre>".print_r($api->getUnsubscribes($this->campaign->CampaignID), 1)."</pre>";
947
                Requirements::customScript($this->JSHackForPreSections(), "CampaignMonitorStats");
948
                $html .= "</div>";
949
                $this->Content = $html;
950
            }
951
        } else {
952
            return false;
953
        }
954
    }
955
956
    /**
957
     * action
958
     * @param HTTPRequest
959
     */
960
    public function resetsignup($request)
961
    {
962
        Session::clear("CampaignMonitorStartForm_AjaxResult_".$this->ID);
963
        return array();
964
    }
965
966
    /**
967
     * removes all campaigns
968
     * so that they can be re-imported.
969
     */
970
    public function resetoldcampaigns()
971
    {
972
        if (!Permission::check("CMS_ACCESS_CMSMain")) {
973
            Security::permissionFailure($this, _t('Security.PERMFAILURE', ' This page is secured and you need CMS rights to access it. Enter your credentials below and we will send you right along.'));
974
        } else {
975
            DB::query("DELETE FROM \"CampaignMonitorCampaign\";");
976
            DB::query("DELETE FROM \"CampaignMonitorCampaign_Pages\";");
977
            die("old campaigns have been deleted");
0 ignored issues
show
Coding Style Compatibility introduced by
The method resetoldcampaigns() contains an exit expression.

An exit expression should only be used in rare cases. For example, if you write a short command line script.

In most cases however, using an exit expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage.

Loading history...
978
        }
979
    }
980
981
982
    /**
983
     * @return String
984
     */
985
    private function JSHackForPreSections()
986
    {
987
        $js = <<<javascript
988
            jQuery(document).ready(
989
                function(){
990
                    jQuery('pre').hide();
991
                    jQuery('#CampaignMonitorStats').on(
992
                        'click',
993
                        'a',
994
                        function(event){
995
                            event.preventDefault();
996
                            jQuery(this).parent().next('pre').slideToggle();
997
                        }
998
                    );
999
                    jQuery("#MyListStatsAreHere").click();
1000
                }
1001
            );
1002
1003
javascript;
1004
        return $js;
1005
    }
1006
}
1007