Review Request: [RFC] Import VPN connection

[Lamarque Vieira Souza]

Em Tuesday 24 May 2011, Rajeesh K Nambiar escreveu:
> > On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > > settings/config/manageconnectionwidget.cpp, line 519
> > > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
> > > ne519>
> > > 
> > >     Shouldn't you also emit a notification or a popup warning the user
> > >     that the import failed?
> 
> Yes, certainly so. But I'm not sure how exactly to do it. Should I be
> modifying libs/service/notificationmanager.cpp? Could you suggest/point to
> how to implement this?

	No, notificationmanager.cpp is kind of passive, it always waits for 
something to notify but only from the networkinterface. In your case you 
should create the KNotification object and emit it yourself, something like:

#include <KNotification>
#include "../libs/service/events.h" // maybe you will need to change this path

K_GLOBAL_STATIC_WITH_ARGS(KComponentData, s_networkManagementComponentData, 
("networkmanagement", "networkmanagement", 
KComponentData::SkipMainComponentRegistration))

static const int m_iconSize = 48;
//end of global variables

KNotification::event(Event::VpnCertificateImportFailed, i18nc("@info:status 
Notification when importing VPN certificate failed", "Certificate import 
failed"), QPixmap(), 0, KNotification::CloseOnTimeout, 
*s_networkManagementComponentDa
ta)->sendEvent();


	Then in ../libs/service/events.h you declare 
Event::VpnCertificateImportFailed and in ../libs/service/events.cpp define it:
const QString Event::VpnCertificateImportFailed = 
QLatin1String("vpncertificateimportfailed");

	In networkmanagement.notifyrc you add the info about the 
vpncertificateimportfailed notification:

[Event/vpncertificateimportfailed]
Name=VPN certificate import failed
Comment=Importing of VPN certificate has failed


> > On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > > vpnplugins/vpnc/vpnc.cpp, line 139
> > > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18014#file18014li
> > > ne139>
> > > 
> > >     Please remove space at line start. Also, you do not insert the
> > >     password into the connection map if the executable failed to run.
> > >     I guess the VPN connection is not going to work in that case,
> > >     right? If so I think you should abort the import process. Think of
> > >     this function as a (SQL) transation: or everything works or the
> > >     entire import process is aborted.
> 
> VPNC user password is almost uninteresting regarding the pcf file. Most of
> the time it will be empty and the user has to manually input. The
> important bit is Group Password/Encrypted Group Password. IMHO, it would
> be better not to abort the import if password decryption fails, but to
> inform the user; so that he can manually rectify it or contact the
> administrator. Further, it is very rare for cisco-decrypt to fail, and
> guaranteed that it will be installed since vpnc package is required
> dependency for NetworkManager-vpnc which is required for
> kde-plasma-networkmanagement-vpnc package.

	Well, NetworkManager-vpnc is not always installed. I for example did not 
have it installed when I first tried to use VPN in my notebook. It took me a 
while to figure out I should have it installed before I could use VPN. Warning 
about missing dependencies is one thing we could improve in Plasma NM in the 
future.

	Ok then, no need to abort the import, just add a notification about it.
 
> > On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > > settings/config/manageconnectionwidget.cpp, line 528
> > > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
> > > ne528>
> > > 
> > >     If this button is visible you should also emit a notification or a
> > >     popup warning that this has not been implemented yet. Or just hide
> > >     the export button.
> 
> I will try to implement the functionality in next iteration; and if it
> looks very difficult then we can simply hide it.

	Ok.

-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110524/c1ff9ae3/attachment.htm