Skip to content

Support for Multi-Channel Funnels API#8

Open
makkaq wants to merge 1 commit into
nwellnhof:masterfrom
makkaq:master
Open

Support for Multi-Channel Funnels API#8
makkaq wants to merge 1 commit into
nwellnhof:masterfrom
makkaq:master

Conversation

@makkaq
Copy link
Copy Markdown

@makkaq makkaq commented May 30, 2017

I've added support for the Multi-Channel Funnels Reporting API.

if ($self->{realtime}) {
$self->{service} = 'realtime';
} else {
push(@required, qw(start_date end_date));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This happens too late (_param is called after _uri) and causes a test failure.

@nwellnhof
Copy link
Copy Markdown
Owner

nwellnhof commented May 30, 2017

I agree with the approach to replace the realtime flag with a service string. But for a proper solution:

  • Don't let Class::XSAccessor autogenerate the Request constructor.
  • Add a manual implementation of the Request constructor that sets service if realtime is set.
  • Remove the autogenerated realtime accessor in Request.
  • Add a manual read/write realtime accessor that uses service internally for backward compatibility:

Something like the following (untested):

sub new {
    my ($class, %args) = @_;
    if (delete($args{realtime})) {
        $args{service} = 'realtime';
    }
    return bless(\%args, ref($class) || $class);
}

# For backward compatibility.
sub realtime {
    my $self = shift;
    if (@_) {
        my $val = shift;
        if ($val) {
            $self->{service} = 'realtime';
        }
        elsif (defined($self->{service}) && $self->{service} eq 'realtime') {
            $self->{service} = undef;
        }
        return $val;
    }
    else {
        return defined($self->{service}) && $self->{service} eq 'realtime';
    }
}

Then you can revert the change to _param.

Please fix the whitespace errors. It seems like you're using hard tabs or something.

Write a test for the MCF API. You can start from a copy of t/02-realtime.t.

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.

2 participants