Email sign up demo doesn't install on 5.7

Permalink
I downloaded the email_list_signup.zip posted at the bottom of this how-to:

http://www.concrete5.org/documentation/how-tos/developers/concrete5...

Unfortunately, it failed to install. Ironically, the error message tells me to check to make sure the add-on was updated for 5.7. It turns out that the package controller in that zip has not been updated as per the very instructions in the how-to so it won't install.

So... I added the required code to the top of the package controller and it installed and the block was successfully placed on the page. However when I used the sign-up block, I received this error:

Call to undefined method Concrete\Core\Permission\IPService::check()


It's tough enough for an old dog like me to try to learn the new tricks of 5.7 and it's even tougher when the very demo pages that try to tell me how to convert an add-on have errors in them. Is it any wonder that the developer community is getting frustrated?

Has anyone gotten this updated add-on to work?

mhawke
 
Tom0 replied on at Permalink Reply
I agree with this, while I don't know the answer because I'm new to Concrete5. 5.7 feels very unfinished and practically unusable. Though the UI is nice... So it's got that going for it.

I will have a look at learning the classes and I will get involved in the github community to see if there is anything I can patch up.
Shotster replied on at Permalink Reply
Shotster
There have apparently been some changes to the IPService class since that write-up back in April. Try changing line 42 of the block controller from...

if (!$ip->check()) {

...to...

if ($ip->isBanned()) {

...and see if it gets you past that error.

-Steve
mhawke replied on at Permalink Reply
mhawke
Yeah, I got that far and then when it tries to save to the database, it complains that the IP cannot be converted to a string. What's really frustrating is that 5.7 is still very much a beta product (and in some ways alpha) and we are being told that we need to develop add-ons against this ever-changing code base.
Shotster replied on at Permalink Reply
Shotster
FWIW, I submitted a fix for this on github. If accepted, I guess it will be in the next release...

https://github.com/concrete5/concrete5-5.7.0/pull/1325/files...

-Steve
Korvin replied on at Permalink Reply
Korvin
The code is not any more "ever-changing" than it ever has been. We are doing bug fixes and feature adds, breaking changes are few and only effect already broken things. This is a standard workflow; release then fix bugs then release then fix bugs then release then fix bugs.

We appreciate you building new sites with 5.7 so that we can actually identify and quickly resolve issues as you run into them. If this is a chore, I encourage you to stick with 5.6 for the time being and reevaluate the stability at a later point.
mhawke replied on at Permalink Reply
mhawke
Thanks Shotster. That fixed it.

@Korvin, I know you are very good at what you do and you are plugging away to make 5.7 perfect. I cannot in good conscience develop new websites in 5.6 because I'd be doing my clients a disservice. The way I see it, it's the same as building a non-responsive website. Any site that is non-responsive is obsolete the minute it goes live. If I go back to that client a year or two from now and tell them they have to re-build their website to stay current, I will loose that client. I'm not the only one who holds this opinion.
exchangecore replied on at Permalink Reply
exchangecore
The IP classes were rewritten to accommodate IPv6. At the time of doing so 5.7 wasn't even "released" yet, and as such I made a decision to change some methods to be named to something that made more sense since I had to break the class for IPv6 support anyway. ("check" to me seemed extremely ambiguous and in fact did the opposite of what I thought it should do, no ambiguity about "isBanned").

As far as database storage, what used to qualify for storage requirements in the database no longer suffice. Previously, most IP's were stored as integers, which cannot be done any longer because IPv6 is 128 bits, which is more than even unsigned integers. Also, character fields did not allow for up to 45 characters of storage could potentially fail to store IPv6 addresses. Another requirement to the IPv6 of concrete5 was that you be able to do < and > comparisons for an ip address, which finally lead to the storing of IP addresses as blob data.

I did build in the ability to get the data in a textual representation should you feel the need to store it in that manner or for display purposes:
$ip = new \Concrete\Core\Utility\IPAddress('127.0.0.1');
echo $ip->getIp(IPAddress::FORMAT_IP_STRING);


The helper code:https://github.com/concrete5/concrete5-5.7.0/blob/develop/web/concre...
Phallanx replied on at Permalink Reply
Phallanx
:@exchangecore

The PHP team must have been drunk when they thought that when programmers instantiate a class with things like
$ip = new \Concrete\Core\Utility\IPAddress('127.0.0.1');

it was a language improvement.

Surely all that core stuff should be in the global namespace. No?
Korvin replied on at Permalink Reply
Korvin
Definitely not. You can research namespaces here:http://php.net/manual/en/language.namespaces.php...
andrew replied on at Permalink Reply
andrew
Sorry about that. You're very much right – there had been some API changes since that how-to, that aren't reflected in the how-to. The new version of the package actually runs $this->getRequestIP()->getIP() (which inserts fine into the database.)
andrew replied on at Permalink Reply
andrew
I have fixed this problem and have updated the package with a version that has the related IP service fixes discussion in this thread as well.