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

email: Add warning for archive folder for POP #1921

Merged
merged 1 commit into from
Apr 1, 2015

Conversation

greezybacon
Copy link
Contributor

This patch adds an error to the page if unable to configure the archive folder and POP was selected as the mailbox protocol. Generally, POP servers do not support anything but the INBOX

@protich
Copy link
Member

protich commented Apr 1, 2015

@greezybacon -- something about error redirection on post fetch archiving doesn't sit well with me. I think it will be a little confusing. Below is my proposal -- error out early and put the error on the post fetch action.

diff --git a/include/class.email.php b/include/class.email.php
index d38806f..26cd686 100644
--- a/include/class.email.php
+++ b/include/class.email.php
@@ -296,8 +296,12 @@ class Email {

             if(!isset($vars['postfetch']))
                 $errors['postfetch']=__('Indicate what to do with fetched emails');
-            elseif(!strcasecmp($vars['postfetch'],'archive') && !$vars['mail_archivefolder'] )
-                $errors['postfetch']=__('Valid folder required');
+            elseif(!strcasecmp($vars['postfetch'],'archive')) {
+                if ($vars['mail_protocol'] == 'POP')
+                    $errors['postfetch'] =  __('POP mail servers do not support folders');
+                elseif (!$vars['mail_archivefolder'])
+                    $errors['postfetch'] = __('Valid folder required');
+            }
         }

         if($vars['smtp_active']) {

@greezybacon
Copy link
Contributor Author

I don't disagree. However, are we sure that POP does in fact NOT support folder archiving?

@greezybacon
Copy link
Contributor Author

Looks like it doesn't

This patch adds an error to the page if unable to configure the archive
folder and POP was selected as the mailbox protocol. Generally, POP servers
do not support anything but the INBOX
@greezybacon
Copy link
Contributor Author

Updated

protich added a commit that referenced this pull request Apr 1, 2015
email: Add warning for archive folder for POP

Reviewed-By: Peter Rotich <peter@osticket.com>
@protich protich merged commit abb8321 into osTicket:develop Apr 1, 2015
@infectormp
Copy link

Maybe better disable or hide archive folder edit box if POP selected?

@greezybacon
Copy link
Contributor Author

@infectormp I think that would make a nice enhancement proposal

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.

None yet

3 participants