From kopete-devel Wed Nov 14 20:57:35 2007 From: Dennis =?utf-8?q?Nienh=C3=BCser?= Date: Wed, 14 Nov 2007 20:57:35 +0000 To: kopete-devel Subject: Re: [kopete-devel] Prevent creating duplicate accounts Message-Id: <200711142157.35966.dennis () nienhueser ! de> X-MARC-Message: https://marc.info/?l=kopete-devel&m=119507389929836 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--Boundary-00=_/E2OH5VJdrcEFO8" --Boundary-00=_/E2OH5VJdrcEFO8 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Am Samstag, 10. November 2007 19:29:12 schrieb Duncan Mac-Vicar P.: > The amount of duplicated code that it introduces in every protocol is what > makes me nervous. Something I spotted: > > - validateData in Kopete"s ediitaccountwidget is pure virtual > - it is reimplemented by protocols, same with validatePassword > > What I suggest: > - rename validateData to validateProtocolData (or just validateData), as > virtual > make some method in the editaccountwidget (validateData or something else?) > non virtual, put the duplicated account code there, and call the virtuals > validateProtocolData (or validateData if it is the name) and > validatePassword as decorator methods. > > Duncan I implemented a virtual validateAccountId(protocol, username, action) now that can be (and gets) called. We can do some more refactoring for 4.1. Patch is attached, I plan to commit this soon. Regards, Dennis --Boundary-00=_/E2OH5VJdrcEFO8 Content-Type: text/x-diff; charset="utf-8"; name="accountedit-datavalidation.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="accountedit-datavalidation.diff" Index: protocols/yahoo/yahooeditaccount.cpp =================================================================== --- protocols/yahoo/yahooeditaccount.cpp (Revision 736753) +++ protocols/yahoo/yahooeditaccount.cpp (Arbeitskopie) @@ -96,21 +96,19 @@ show(); } -bool YahooEditAccount::validateData() +bool YahooEditAccount::validateData( Kopete::AccountChangeAction changeAction ) { kDebug(YAHOO_GEN_DEBUG) ; - if(mScreenName->text().isEmpty()) - { KMessageBox::queuedMessageBox(this, KMessageBox::Sorry, - i18n("You must enter a valid screen name."), i18n("Yahoo")); - return false; - } + QString userName(mScreenName->text()); + if(!mPasswordWidget->validate()) { KMessageBox::queuedMessageBox(this, KMessageBox::Sorry, i18n("You must enter a valid password."), i18n("Yahoo")); return false; } - return true; + + return validateAccountId("YahooProtocol", userName, changeAction); } Kopete::Account *YahooEditAccount::apply() Index: protocols/yahoo/yahooeditaccount.h =================================================================== --- protocols/yahoo/yahooeditaccount.h (Revision 736753) +++ protocols/yahoo/yahooeditaccount.h (Arbeitskopie) @@ -43,7 +43,7 @@ public: YahooEditAccount(YahooProtocol *protocol, Kopete::Account *theAccount, QWidget *parent = 0); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); public slots: virtual Kopete::Account *apply(); Index: protocols/gadu/gadueditaccount.h =================================================================== --- protocols/gadu/gadueditaccount.h (Revision 736753) +++ protocols/gadu/gadueditaccount.h (Arbeitskopie) @@ -38,7 +38,7 @@ public: GaduEditAccount( GaduProtocol*, Kopete::Account*, QWidget* parent = 0 ); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); Kopete::Account* apply(); private slots: Index: protocols/gadu/gadueditaccount.cpp =================================================================== --- protocols/gadu/gadueditaccount.cpp (Revision 736753) +++ protocols/gadu/gadueditaccount.cpp (Arbeitskopie) @@ -210,15 +210,13 @@ } bool -GaduEditAccount::validateData() +GaduEditAccount::validateData( Kopete::AccountChangeAction changeAction ) { - - if ( loginEdit_->text().isEmpty() ) { - KMessageBox::sorry( this, i18n( "Enter UIN please." ), i18n( "Gadu-Gadu" ) ); + QString userName(loginEdit_->text()); + if ( !validateAccountId("GaduProtocol", userName, changeAction) ) return false; - } - if ( loginEdit_->text().toInt() < 0 || loginEdit_->text().toInt() == 0 ) { + if ( userName.toInt() <= 0 ) { KMessageBox::sorry( this, i18n( "UIN should be a positive number." ), i18n( "Gadu-Gadu" ) ); return false; } Index: protocols/winpopup/wpeditaccount.cpp =================================================================== --- protocols/winpopup/wpeditaccount.cpp (Revision 736753) +++ protocols/winpopup/wpeditaccount.cpp (Arbeitskopie) @@ -36,7 +36,6 @@ #include #include - // Kopete Includes #include @@ -93,11 +92,12 @@ mProtocol->installSamba(); } -bool WPEditAccount::validateData() +bool WPEditAccount::validateData( Kopete::AccountChangeAction changeAction ) { kDebug(14170) << "WPEditAccount::validateData()"; - if(mHostName->text().isEmpty()) { + QString hostname(mHostName->text()); + if(hostname.isEmpty()) { KMessageBox::sorry(this, i18n("You must enter a valid screen name."), i18n("WinPopup")); return false; } @@ -108,7 +108,7 @@ return false; } - return true; + return validateAccountId("WPProtocol", hostname, changeAction); } void WPEditAccount::writeConfig() Index: protocols/winpopup/wpeditaccount.h =================================================================== --- protocols/winpopup/wpeditaccount.h (Revision 736753) +++ protocols/winpopup/wpeditaccount.h (Arbeitskopie) @@ -43,7 +43,7 @@ public: WPEditAccount(QWidget *parent, Kopete::Account *theAccount); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); void writeConfig(); public slots: Index: protocols/qq/ui/qqeditaccountwidget.cpp =================================================================== --- protocols/qq/ui/qqeditaccountwidget.cpp (Revision 736753) +++ protocols/qq/ui/qqeditaccountwidget.cpp (Arbeitskopie) @@ -165,14 +165,21 @@ return account(); } -bool QQEditAccountWidget::validateData() +bool QQEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { QString userid = d->ui->m_login->text(); + + if ( !validateAccountId("QQProtocol", userid, changeAction) ) + { + return false; + } + if ( QQProtocol::validContactId( userid ) ) return true; KMessageBox::queuedMessageBox( Kopete::UI::Global::mainWidget(), KMessageBox::Sorry, i18n( "You must enter a valid email address." ), i18n( "QQ Plugin" ) ); + return false; } Index: protocols/qq/ui/qqeditaccountwidget.h =================================================================== --- protocols/qq/ui/qqeditaccountwidget.h (Revision 736753) +++ protocols/qq/ui/qqeditaccountwidget.h (Arbeitskopie) @@ -42,7 +42,7 @@ /** * Is the data correct? */ - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); private slots: void slotOpenRegister(); Index: protocols/groupwise/ui/gweditaccountwidget.cpp =================================================================== --- protocols/groupwise/ui/gweditaccountwidget.cpp (Revision 736753) +++ protocols/groupwise/ui/gweditaccountwidget.cpp (Arbeitskopie) @@ -114,9 +114,12 @@ return account(); } -bool GroupWiseEditAccountWidget::validateData() +bool GroupWiseEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { - return !( m_preferencesDialog->m_userId->text().isEmpty() || m_preferencesDialog->m_server->text().isEmpty() ); + if (!validateAccountId("GroupwiseProtocol", m_preferencesDialog->m_userId->text(), changeAction)) + return false; + + return !m_preferencesDialog->m_server->text().isEmpty(); } void GroupWiseEditAccountWidget::writeConfig() Index: protocols/groupwise/ui/gweditaccountwidget.h =================================================================== --- protocols/groupwise/ui/gweditaccountwidget.h (Revision 736753) +++ protocols/groupwise/ui/gweditaccountwidget.h (Arbeitskopie) @@ -49,7 +49,7 @@ /** * Is the data correct? */ - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); protected slots: void configChanged(); protected: Index: protocols/jabber/ui/jabbereditaccountwidget.cpp =================================================================== --- protocols/jabber/ui/jabbereditaccountwidget.cpp (Revision 736753) +++ protocols/jabber/ui/jabbereditaccountwidget.cpp (Arbeitskopie) @@ -194,11 +194,17 @@ account()->configGroup()->writeEntry("HideSystemInfo", cbHideSystemInfo->isChecked()); } -bool JabberEditAccountWidget::validateData () +bool JabberEditAccountWidget::validateData ( Kopete::AccountChangeAction changeAction ) { + QString userId(mID->text()); - if(!mID->text().contains('@')) + if (!validateAccountId("JabberProtocol", userId, changeAction)) { + return false; + } + + if(!userId.contains('@')) + { KMessageBox::sorry(this, i18n("The Jabber ID you have chosen is invalid. " "Please make sure it is in the form user@server.com, like an email address."), i18n("Invalid Jabber ID")); Index: protocols/jabber/ui/jabbereditaccountwidget.h =================================================================== --- protocols/jabber/ui/jabbereditaccountwidget.h (Revision 736753) +++ protocols/jabber/ui/jabbereditaccountwidget.h (Arbeitskopie) @@ -39,7 +39,7 @@ public: JabberEditAccountWidget (JabberProtocol * proto, JabberAccount *, QWidget * parent = 0); ~JabberEditAccountWidget (); - virtual bool validateData (); + virtual bool validateData ( Kopete::AccountChangeAction changeAction=Kopete::Create ); virtual Kopete::Account *apply (); JabberAccount *account (); Index: protocols/testbed/testbededitaccountwidget.cpp =================================================================== --- protocols/testbed/testbededitaccountwidget.cpp (Revision 736753) +++ protocols/testbed/testbededitaccountwidget.cpp (Arbeitskopie) @@ -58,7 +58,7 @@ return account(); } -bool TestbedEditAccountWidget::validateData() +bool TestbedEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { //return !( m_preferencesWidget->m_acctName->text().isEmpty() ); return true; Index: protocols/testbed/testbededitaccountwidget.h =================================================================== --- protocols/testbed/testbededitaccountwidget.h (Revision 736753) +++ protocols/testbed/testbededitaccountwidget.h (Arbeitskopie) @@ -45,7 +45,7 @@ /** * Is the data correct? */ - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); protected: Kopete::Account *m_account; Ui::TestbedAccountPreferences *m_preferencesWidget; Index: protocols/msn/ui/msneditaccountwidget.cpp =================================================================== --- protocols/msn/ui/msneditaccountwidget.cpp (Revision 736753) +++ protocols/msn/ui/msneditaccountwidget.cpp (Arbeitskopie) @@ -214,9 +214,15 @@ return account(); } -bool MSNEditAccountWidget::validateData() +bool MSNEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { QString userid = d->ui->m_login->text(); + + if ( !validateAccountId("MSNProtocol", userid, changeAction) ) + { + return false; + } + if ( MSNProtocol::validContactId( userid ) ) return true; Index: protocols/msn/ui/msneditaccountwidget.h =================================================================== --- protocols/msn/ui/msneditaccountwidget.h (Revision 736753) +++ protocols/msn/ui/msneditaccountwidget.h (Arbeitskopie) @@ -39,7 +39,7 @@ public: MSNEditAccountWidget( MSNProtocol *proto, Kopete::Account *account, QWidget *parent = 0 ); ~MSNEditAccountWidget(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); virtual Kopete::Account * apply(); private slots: Index: protocols/telepathy/ui/telepathyeditaccountwidget.cpp =================================================================== --- protocols/telepathy/ui/telepathyeditaccountwidget.cpp (Revision 736753) +++ protocols/telepathy/ui/telepathyeditaccountwidget.cpp (Arbeitskopie) @@ -105,7 +105,7 @@ return static_cast( KopeteEditAccountWidget::account() ); } -bool TelepathyEditAccountWidget::validateData() +bool TelepathyEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { kDebug(TELEPATHY_DEBUG_AREA) ; // You must fill the form to move to the next step Index: protocols/telepathy/ui/telepathyeditaccountwidget.h =================================================================== --- protocols/telepathy/ui/telepathyeditaccountwidget.h (Revision 736753) +++ protocols/telepathy/ui/telepathyeditaccountwidget.h (Arbeitskopie) @@ -42,7 +42,7 @@ explicit TelepathyEditAccountWidget(Kopete::Account *account, QWidget *parent = 0); ~TelepathyEditAccountWidget(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); /** * Create a new account if we are in the 'add account wizard', Index: protocols/sms/smseditaccountwidget.h =================================================================== --- protocols/sms/smseditaccountwidget.h (Revision 736753) +++ protocols/sms/smseditaccountwidget.h (Arbeitskopie) @@ -33,7 +33,7 @@ SMSEditAccountWidget(SMSProtocol *protocol, Kopete::Account *theAccount, QWidget *parent = 0); ~SMSEditAccountWidget(); - bool validateData(); + bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); Kopete::Account* apply(); public slots: void setServicePreferences(const QString& serviceName); Index: protocols/sms/smseditaccountwidget.cpp =================================================================== --- protocols/sms/smseditaccountwidget.cpp (Revision 736753) +++ protocols/sms/smseditaccountwidget.cpp (Arbeitskopie) @@ -90,9 +90,9 @@ delete service; } -bool SMSEditAccountWidget::validateData() +bool SMSEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { - return true; + return validateAccountId("SMSProtocol", preferencesDialog->accountId->text(), changeAction); } Kopete::Account* SMSEditAccountWidget::apply() Index: protocols/oscar/aim/ui/aimeditaccountwidget.h =================================================================== --- protocols/oscar/aim/ui/aimeditaccountwidget.h (Revision 736753) +++ protocols/oscar/aim/ui/aimeditaccountwidget.h (Arbeitskopie) @@ -44,7 +44,7 @@ QWidget *parent=0); virtual ~AIMEditAccountWidget(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); virtual Kopete::Account *apply(); private slots: Index: protocols/oscar/aim/ui/aimeditaccountwidget.cpp =================================================================== --- protocols/oscar/aim/ui/aimeditaccountwidget.cpp (Revision 736753) +++ protocols/oscar/aim/ui/aimeditaccountwidget.cpp (Arbeitskopie) @@ -203,7 +203,7 @@ return mAccount; } -bool AIMEditAccountWidget::validateData() +bool AIMEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { //kDebug(14152) << "Called."; @@ -211,18 +211,13 @@ QString server = mGui->edtServerAddress->text(); int port = mGui->sbxServerPort->value(); - if ( userName.length() < 1 ) - return false; - if ( port < 1 ) return false; if ( server.length() < 1 ) return false; - // Seems good to me - //kDebug(14152) << "Account data validated successfully."; - return true; + return validateAccountId("AIMProtocol", userName, changeAction); } void AIMEditAccountWidget::slotOpenRegister() Index: protocols/oscar/icq/icqprotocol.cpp =================================================================== --- protocols/oscar/icq/icqprotocol.cpp (Revision 736753) +++ protocols/oscar/icq/icqprotocol.cpp (Arbeitskopie) @@ -842,6 +842,11 @@ return statusManager_; } +bool ICQProtocol::validatePassword( const QString & password ) const +{ + return password.length() >= 6 && password.length() <= 8; +} + //END class ICQProtocol #include "icqprotocol.moc" Index: protocols/oscar/icq/icqprotocol.h =================================================================== --- protocols/oscar/icq/icqprotocol.h (Revision 736753) +++ protocols/oscar/icq/icqprotocol.h (Arbeitskopie) @@ -76,6 +76,8 @@ void setTZComboValue(QComboBox *combo, const char &tz); char getTZComboValue(QComboBox *combo); */ + virtual bool validatePassword( const QString & password ) const; + private: void initGenders(); void initLang(); Index: protocols/oscar/icq/ui/icqeditaccountwidget.cpp =================================================================== --- protocols/oscar/icq/ui/icqeditaccountwidget.cpp (Revision 736753) +++ protocols/oscar/icq/ui/icqeditaccountwidget.cpp (Arbeitskopie) @@ -68,6 +68,8 @@ QRegExp rx("[0-9]{9}"); QValidator* validator = new QRegExpValidator( rx, this ); mAccountSettings->edtAccountId->setValidator(validator); + + mAccountSettings->mPasswordWidget->setValidationProtocol(protocol); // Read in the settings from the account if it exists if(mAccount) @@ -78,7 +80,7 @@ mAccountSettings->edtAccountId->setReadOnly(true); mAccountSettings->mPasswordWidget->load(&mAccount->password()); mAccountSettings->chkAutoLogin->setChecked(mAccount->excludeConnect()); - + QString serverEntry = mAccount->configGroup()->readEntry("Server", "login.oscar.aol.com"); int portEntry = mAccount->configGroup()->readEntry("Port", 5190); if ( serverEntry != "login.oscar.aol.com" || ( portEntry != 5190) ) @@ -261,7 +263,7 @@ return mAccount; } -bool ICQEditAccountWidget::validateData() +bool ICQEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { kDebug(14153) << "Called."; bool bOk; @@ -274,6 +276,18 @@ return false; } + if ( !validateAccountId("ICQProtocol", userId, changeAction) ) + { + return false; + } + + if (!mAccountSettings->mPasswordWidget->validate()) + { + KMessageBox::queuedMessageBox( this, KMessageBox::Sorry, i18nc( "@info", + "The password entered does not match the ICQ password guidelines: Passwords must be 6-8 characters long."), i18n( "ICQ" ) ); + return false; + } + // No need to check port, min and max values are properly defined in .ui if (mAccountSettings->edtServerAddress->text().isEmpty()) Index: protocols/oscar/icq/ui/icqeditaccountwidget.h =================================================================== --- protocols/oscar/icq/ui/icqeditaccountwidget.h (Revision 736753) +++ protocols/oscar/icq/ui/icqeditaccountwidget.h (Arbeitskopie) @@ -40,7 +40,7 @@ QWidget *parent=0); ~ICQEditAccountWidget(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); virtual Kopete::Account *apply(); private slots: Index: protocols/meanwhile/meanwhileeditaccountwidget.cpp =================================================================== --- protocols/meanwhile/meanwhileeditaccountwidget.cpp (Revision 736753) +++ protocols/meanwhile/meanwhileeditaccountwidget.cpp (Arbeitskopie) @@ -83,15 +83,11 @@ return myAccount; } -bool MeanwhileEditAccountWidget::validateData() +bool MeanwhileEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { - if(mScreenName->text().isEmpty()) - { - KMessageBox::queuedMessageBox(this, KMessageBox::Sorry, - i18n("You must enter a valid screen name."), - i18n("Meanwhile Plugin")); + if(!validateAccountId("MeanwhileProtocol", mScreenName->text(), changeAction)) return false; - } + if( !mPasswordWidget->validate() ) { KMessageBox::queuedMessageBox(this, KMessageBox::Sorry, Index: protocols/meanwhile/meanwhileeditaccountwidget.h =================================================================== --- protocols/meanwhile/meanwhileeditaccountwidget.h (Revision 736753) +++ protocols/meanwhile/meanwhileeditaccountwidget.h (Arbeitskopie) @@ -40,7 +40,7 @@ virtual Kopete::Account* apply(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); protected slots: void slotSetServer2Default(); protected: Index: protocols/irc/ui/irceditaccountwidget.cpp =================================================================== --- protocols/irc/ui/irceditaccountwidget.cpp (Revision 736753) +++ protocols/irc/ui/irceditaccountwidget.cpp (Arbeitskopie) @@ -268,7 +268,7 @@ return account(); } -bool IRCEditAccountWidget::validateData() +bool IRCEditAccountWidget::validateData( Kopete::AccountChangeAction changeAction ) { if( nickNames->text().isEmpty() ) KMessageBox::sorry(this, i18n("You must enter a nickname."), i18n("Kopete")); Index: protocols/irc/ui/irceditaccountwidget.h =================================================================== --- protocols/irc/ui/irceditaccountwidget.h (Revision 736753) +++ protocols/irc/ui/irceditaccountwidget.h (Arbeitskopie) @@ -37,7 +37,7 @@ ~IRCEditAccountWidget(); IRCAccount *account(); - virtual bool validateData(); + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ); virtual Kopete::Account *apply(); private slots: Index: kopete/addaccountwizard/addaccountwizard.cpp =================================================================== --- kopete/addaccountwizard/addaccountwizard.cpp (Revision 736753) +++ kopete/addaccountwizard/addaccountwizard.cpp (Arbeitskopie) @@ -180,7 +180,7 @@ else if (currentPage()->widget() == d->accountPageWidget) { // check the data of the page is valid - if (!d->accountPage->validateData()) + if (!d->accountPage->validateData(Kopete::Create)) { return; } @@ -201,17 +201,33 @@ void AddAccountWizard::accept() { - // registeredAccount shouldn't probably be called here. Anyway, if the account is already registered, - // it won't be registered twice Kopete::AccountManager *manager = Kopete::AccountManager::self(); - Kopete::Account *account = manager->registerAccount(d->accountPage->apply()); - // if the account wasn't created correctly then leave + // Have the plugin extract account information + Kopete::Account *account = d->accountPage->apply(); if (!account) { + KMessageBox::sorry( this, i18nc("@info", "An error occurred during account creation.")); return; } + // Check that the account doesn't exist yet, otherwise the account manager would delete it during registration + // This shouldn't be the case as the protocol's account edit page should prevent this, but it's not guaranteed + if (manager->findAccount(account->protocol()->pluginId(), account->accountId())) + { + KMessageBox::sorry( this, i18nc("@info", "This account already exists.")); + reject(); + return; + } + + // Register account and leave if it doesn't work + account = manager->registerAccount(account); + if (!account) + { + KMessageBox::sorry( this, i18nc("@info", "An error occurred during account registration.")); + return; + } + // Make sure the protocol is correctly enabled. This is not really needed, but still good const QString PROTO_NAME = d->proto->pluginId().remove("Protocol").toLower(); Kopete::PluginManager::self()->setPluginEnabled(PROTO_NAME , true); Index: kopete/config/accounts/kopeteaccountconfig.cpp =================================================================== --- kopete/config/accounts/kopeteaccountconfig.cpp (Revision 736753) +++ kopete/config/accounts/kopeteaccountconfig.cpp (Arbeitskopie) @@ -239,6 +239,9 @@ void KopeteAccountConfig::modifyAccount(Kopete::Account *account) { + // FIXME: This function is almost identical to editAccount() in libkopete/kopeteaccount.cpp + // and both should be merged. + Kopete::Protocol *proto = account->protocol(); KDialog editDialog ( this ); @@ -264,7 +267,11 @@ editDialog.setMainWidget( w ); if ( editDialog.exec() == QDialog::Accepted ) { - if( m_accountWidget->validateData() ) + // FIXME: The dialog is hidden at this point, which means that messages + // created in validateData() with "this" as parent won't be shown. It + // should be called earlier such that messages are displayed to the user + // and he gets a chance to know about them and fix them instantly. + if( m_accountWidget->validateData(Kopete::Edit) ) m_accountWidget->apply(); } Index: libkopete/kopeteaccount.cpp =================================================================== --- libkopete/kopeteaccount.cpp (Revision 736753) +++ libkopete/kopeteaccount.cpp (Arbeitskopie) @@ -566,11 +566,11 @@ void Account::editAccount(QWidget *parent) { - KDialog *editDialog = new KDialog( parent ); - editDialog->setCaption( i18n( "Edit Account" ) ); - editDialog->setButtons( KDialog::Ok | KDialog::Apply | KDialog::Cancel ); + KDialog editDialog( parent ); + editDialog.setCaption( i18n( "Edit Account" ) ); + editDialog.setButtons( KDialog::Ok | KDialog::Cancel ); - KopeteEditAccountWidget *m_accountWidget = protocol()->createEditAccountWidget( this, editDialog ); + KopeteEditAccountWidget *m_accountWidget = protocol()->createEditAccountWidget( this, &editDialog ); if ( !m_accountWidget ) return; @@ -584,14 +584,16 @@ if ( !w ) return; - editDialog->setMainWidget( w ); - if ( editDialog->exec() == QDialog::Accepted ) + editDialog.setMainWidget( w ); + if ( editDialog.exec() == QDialog::Accepted ) { - if( m_accountWidget->validateData() ) + // FIXME: The dialog is hidden at this point, which means that messages + // created in validateData() with "this" as parent won't be shown. It + // should be called earlier such that messages are displayed to the user + // and he gets a chance to know about them and fix them instantly. + if( m_accountWidget->validateData(Kopete::Edit) ) m_accountWidget->apply(); } - - editDialog->deleteLater(); } void Account::setCustomIcon( const QString & i) Index: libkopete/ui/editaccountwidget.cpp =================================================================== --- libkopete/ui/editaccountwidget.cpp (Revision 736753) +++ libkopete/ui/editaccountwidget.cpp (Arbeitskopie) @@ -17,7 +17,12 @@ */ #include "editaccountwidget.h" +#include +#include +#include +#include + class KopeteEditAccountWidgetPrivate { public: @@ -45,5 +50,23 @@ d->account = account; } +bool KopeteEditAccountWidget::validateAccountId( const QString &protocolId, const QString &accountId, Kopete::AccountChangeAction changeAction ) +{ + if ( accountId.isEmpty() ) + { + KMessageBox::queuedMessageBox( Kopete::UI::Global::mainWidget(), KMessageBox::Sorry, i18nc( "@info", + "Please fill in an account ID.") ); + return false; + } + + if (changeAction == Kopete::Create && Kopete::AccountManager::self()->findAccount( protocolId, accountId ) ) + { + KMessageBox::queuedMessageBox( Kopete::UI::Global::mainWidget(), KMessageBox::Sorry, i18nc( "@info", + "This account already exists. Please choose a different account ID or use the Configure Dialog to change the settings of the %1 account.", accountId) ); + return false; + } + + return true; +} + // vim: set noet ts=4 sts=4 sw=4: - Index: libkopete/ui/editaccountwidget.h =================================================================== --- libkopete/ui/editaccountwidget.h (Revision 736753) +++ libkopete/ui/editaccountwidget.h (Arbeitskopie) @@ -23,7 +23,17 @@ namespace Kopete { -class Account; + class Account; + + /** + * \brief Possible changes to accounts + * @sa @ref KopeteEditAccountWidget::validateData() + */ + enum AccountChangeAction { + Create = 0, ///< New account will be created + Edit = 1 ///< Existing account will be changed + }; + } class KopeteEditAccountWidgetPrivate; @@ -70,9 +80,11 @@ /** * This method must be reimplemented. - * It does the same as @ref AddContactPage::validateData() + * Returns true if all account configuration data filled in by the user is valid, and + * false otherwise, preferably with a message telling what is wrong. + * It is similar to @ref AddContactPage::validateData() */ - virtual bool validateData() = 0; + virtual bool validateData( Kopete::AccountChangeAction changeAction=Kopete::Create ) = 0; /** * Create a new account if we are in the 'add account wizard', @@ -80,6 +92,12 @@ */ virtual Kopete::Account *apply() = 0; + /** + * Account id (username) validation. The default implementation returns false if the account + * id is empty or the account already exists in case of changeAction==Create. + */ + virtual bool validateAccountId( const QString &protocolId, const QString &accountId, Kopete::AccountChangeAction changeAction ); + protected: /** * Get a pointer to the Kopete::Account passed to the constructor. --Boundary-00=_/E2OH5VJdrcEFO8 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel --Boundary-00=_/E2OH5VJdrcEFO8--