D17834: Allow the use of One-Time Password

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> fortisslvpnauth.cpp:44
> +    const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());
> +    if (otpFlag == NetworkManager::Setting::NotSaved){
> +        d->ui.otpFrame->setVisible(true);

Missing space before "{".

> fortisslvpnauth.cpp:47
> +    } else {
> +        d->ui.otpFrame->setVisible(false);
> +    }

You can make this shorter:
d->ui.otpFrame->setVisible(otpFlag == NetworkManager::Setting::NotSaved)

> fortisslvpnauth.cpp:69
> +    
> +    if (!data.value(NM_FORTISSLVPN_KEY_OTP"-flags").isEmpty()){
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());

Missing space before "{".

> fortisslvpnauth.cpp:71
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());
> +        if (otpFlag == NetworkManager::Setting::NotSaved && !d->ui.otp->text().isEmpty()){
> +            secrets.insert(QLatin1String(NM_FORTISSLVPN_KEY_OTP),d->ui.otp->text());

Missing space before "{".

> fortisslvpnauth.cpp:72
> +        if (otpFlag == NetworkManager::Setting::NotSaved && !d->ui.otp->text().isEmpty()){
> +            secrets.insert(QLatin1String(NM_FORTISSLVPN_KEY_OTP),d->ui.otp->text());
> +        }

Missing space after second parameter.

> fortisslvpnwidget.cpp:131
>  
> +    if (!data.value(NM_FORTISSLVPN_KEY_OTP"-flags").isEmpty()){
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());

Missing space before "{".

> fortisslvpnwidget.cpp:133
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());
> +        if (otpFlag & NetworkManager::Setting::NotSaved){
> +            d->advUi.otp->setChecked(true);

Missing space before "{".

> fortisslvpnwidget.cpp:201
> +    const NMStringMap prevData = d->setting->data();
> +    if (!prevData.value(NM_FORTISSLVPN_KEY_OTP"-flags").isEmpty()){
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());

Missing space before "{".

> fortisslvpnwidget.cpp:201
> +    const NMStringMap prevData = d->setting->data();
> +    if (!prevData.value(NM_FORTISSLVPN_KEY_OTP"-flags").isEmpty()){
> +        const NetworkManager::Setting::SecretFlags otpFlag = static_cast<NetworkManager::Setting::SecretFlags>(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt());

I don't understand why you hare check for prevData.value(), but later on you get optFlag from data (not prevData). Still I don't think this is needed at all. I would go back and use your previous approach, because the result will be the same.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D17834

To: enriquem, jgrulich
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190105/3e7371c1/attachment-0001.html>