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

Issue #889 Add Validation when Adding Mail Forward #7

Merged
merged 6 commits into from
Feb 23, 2012
Merged

Issue #889 Add Validation when Adding Mail Forward #7

merged 6 commits into from
Feb 23, 2012

Conversation

andrewying
Copy link
Contributor

As the topic, this is a bugfix pull request for #889 at http://project.lxcenter.org/issues/889. This add validation to the script for adding a mail forward.

Remark: this does not do any cleanup.

@ghost ghost assigned shakaran Feb 22, 2012
}
else if ((substr($param['forwardaddress'], 0, 1) != '|')
&& (!validate_email($param['forwardaddress']))) {
throw new lxException('forwardaddress_invaild', 'forwardaddress');
Copy link
Contributor

Choose a reason for hiding this comment

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

s/forwardaddress_invaild/forwardaddress_invalid

@shazarlx
Copy link
Contributor

As Rene wrote in the issue thread, it seems like this would break this feature: http://wiki.lxcenter.org/Kloxo+Email+Piping

Any thoughts?

@shazarlx
Copy link
Contributor

Ah nevermind, I re-read it and it looks like it takes that into consideration.

@shazarlx
Copy link
Contributor

Oops nevermind, that is an exception to the rule:) Carry on.

@andrewying
Copy link
Contributor Author

@shazarlx OK I have checked and see the validation on single quotes has already been carried out. Not necessary to keep that code anyway.

@shakaran I have not put underscore now. But where should I put the "InvalidForwardAddressException"? We still have not have it set yet. I will put it in the language file at this moment though.

@shakaran
Copy link
Contributor

@andrewying It is more useful recieve a exception named. If you calls to the method, you can recieve a lot lxException different and you don't have a properly way to treat a particular exception.

For example, if you want treat on different way forwardaddress_cannot_empty and forwardaddress_invaild.

A lxException only:

function updateform()
{
    // Bla bla
    if(empty($something) {
        throw new lxException('forwardaddress_cannot_empty', 'forwardaddress');
    }
    elseif($something != 5) {
        throw new lxException('forwardaddress_invaild', 'forwardaddress');
    }
}

try
{
    updateform()
}
catch (lxException $e) {
    echo 'Something is wrong, but good luck parsing $e for every type of lxException';
}

The smart way with subclassing inherit:

class EmptyForwardAddressException extends lxException { }
class InvalidForwardAddressException extends lxException 
{ 
    function reportTest() 
    { 
        echo 'Reporting';
    }
}

function updateform()
{
    // Bla bla
    if(empty($something) {
        throw new EmptyForwardAddressException('forwardaddress_cannot_empty', 'forwardaddress');
    }
    elseif($something != 5) {
        throw new InvalidForwardAddressException('forwardaddress_invaild', 'forwardaddress');
    }
}

try
{
    updateform()
}
catch (EmptyForwardAddressException $e) {
    echo 'Oh men! You left empty your forward address';
    askAgain();
}
catch (InvalidForwardAddressException $e) {
    echo 'Bad guy! Reporting to admin!';
    reportingToAdmin();
    $e->reportTest(); // See here custom treatment depending of a exception if it is needed
}

@andrewying
Copy link
Contributor Author

Well adding this to the language file still had the same effect

Best Regards,
Andrew Ying
Sent from my iPhone

On 23 Feb, 2012, at 6:08 PM, Ángel Guzmán Maesoreply@reply.github.com wrote:

@andrewying It is more useful recieve a exception named. If you calls to the method, you can recieve a lot lxException different and you don't have a properly way to treat a particular exception.

For example, if you want treat on different way forwardaddress_cannot_empty and forwardaddress_invaild.

A lxException only:

function updateform()
{
   // Bla bla
   if(empty($something) {
       throw new lxException('forwardaddress_cannot_empty', 'forwardaddress');
   }
   elseif($something != 5) {
       throw new lxException('forwardaddress_invaild', 'forwardaddress');
   }
}

try
{
   updateform()
}
catch (lxException $e) {
   echo 'Something is wrong, but good luck parsing $e for every type of lxException';
}

The smart way with subclassing inherit:

class EmptyForwardAddressException extends lxException { }
class InvalidForwardAddressException extends lxException 
{ 
   function reportTest() 
   { 
       echo 'Reporting';
   }
}

function updateform()
{
   // Bla bla
   if(empty($something) {
       throw new EmptyForwardAddressException('forwardaddress_cannot_empty', 'forwardaddress');
   }
   elseif($something != 5) {
       throw new InvalidForwardAddressException('forwardaddress_invaild', 'forwardaddress');
   }
}

try
{
   updateform()
}
catch (EmptyForwardAddressException $e) {
   echo 'Oh men! You left empty your forward address';
   askAgain();
}
catch (InvalidForwardAddressException $e) {
   echo 'Bad guy! Reporting to admin!';
   reportingToAdmin();
   $e->reportTest(); // See here custom treatment depending of a exception if it is needed
}

Reply to this email directly or view it on GitHub:
#7 (comment)

@shakaran
Copy link
Contributor

I just suggest that way for a future (it is not needed on this example for works neccesary better). Just change the typo when you can and we can move forward for accept the pull request.

@andrewying
Copy link
Contributor Author

@shakaran Yes actually I agree on your idea, but for now using language files is more efficient and actually more suitable as this issue is for 6.1.x release. BTW, I have updated the commit.

@shakaran
Copy link
Contributor

Did you forget push the new commit? Remember: git push origin bug-889

@andrewying
Copy link
Contributor Author

@shakaran Check out the three commits committed today. It is between the discussions. And for your reference they are 4849c8f, 2172e25 and 02aaf8c

shakaran added a commit that referenced this pull request Feb 23, 2012
Issue #889 Add Validation when Adding Mail Forward
@shakaran shakaran merged commit 8e2afbe into lxcenter:dev Feb 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants