On 06/18/2015 12:52 PM, Jan Cholasta wrote:
Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a):
Dne 15.6.2015 v 17:29 thierry bordaz napsal(a):
On 06/15/2015 05:00 PM, Simo Sorce wrote:
On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote:
On 06/09/2015 02:02 PM, Jan Cholasta wrote:
Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a):
Dne 18.5.2015 v 10:33 thierry bordaz napsal(a):
On 05/15/2015 04:44 PM, David Kupka wrote:
Hello Thierry,
thanks for the patch set. Overall functionality of ULC feature
good to
me and is definitely "alpha ready".

I found following issues but don't insist on fixing it right now:

1) When stageuser-activate fails due to already existent
active/deleted user.
DN is show instead of user name that's used in other commands
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user

already exists
Hi David, Jan,

Thanks you so much for all those tests and feedback. I agree, some
bugs can be fixed separatly from this main patches.

You are right, It should return the user ID not the DN.

2) According to the design there should be '--only-delete' and
options for user-find command instead there is '--preserved'
Honza proposed adding virtual boolean attribute 'deleted' to user
entry and
filter on it.
The 'deleted' attribute would be useful also in user-show where
is no
way to
tell if the displayed user is active or deleted. (Except running
and looking on the dn).
Yes a bit late to resynch the design.
The final option is 'preserved' for user-find and 'preserve' for
user-del. '--only-delete' or 'also-delete' are old name that I
need to
replace in the design.

About the 'deleted' attribute, do you think adding a DS cos virtual
attribute ?
See the attached patch.
Can someone please review the patch?

3) uidNumber and gidNumber can't be set back to '-1' once set to
This would be useful when admin changes its mind and want IPA to
assign them.
IIUC, there should be no validation in cn=staged user container.
validation should be done during stageuser-activate.
Yes that comes from user plugin that enforce the number to be >0.
That is a good point giving the ability to reset
I will check if it is possible, how (give a value or an option to
reset), and also if it would not create other issue.
4) Support for deleted -> stage workflow is still missing. But I'm
unsure if we
agreed to finish it now or later.
Yes thanks
5) Twice deleting user with '--preserve' deletes him permanently.
$ ipa user-add tuser --first Test --last User
$ ipa user-del tuser --preserve
$ ipa user-del tuser --preserve
$ ipa user-find --preserved
0 (delete) users matched
Number of entries returned 0
Deleting a deleted (preserved) entry, should permanently remove the
+1, but no-op if default behavior is "preserve"

Now if the second time the preserve option is present, it makes
sense to
not delete it.
+1, should be no-op

BTW: I might be stating the obvious here, but it would be better to
one boolean parameter rather than two mutually exclusive flags in
I would like an opinion on this as well.

So the proposal is, e.g.,:

    ipa user del fbar --preserve
    ipa user del fbar --permanently
    ipa user del fbar --permanently=False
    ipa user del fbar --permanently=True
    ipa user del fbar
uses the default behavior(permanently atm.)

I don't think there is a big difference. A boolean is easier for
scripting. 2 options are more descriptive for humans. With a single
boolean, I would be afraid that omitting it would imply False to some
users which is not always the same as "the default behavior" [1].

With Web UI developer hat I would vote for single boolean but as a CLI
user I would like the current options.

Given that Web UI or any other API client should not define CLI, I
keep the current options.

my 2c

Petr Vobornik

+1 --preserve is 100x better for a human than --permanently=False

I also prefere --preserve for usability of  'user del'.

In addition we have 'user find|show --preserved' to retrieve users that
have been preserved. So it seems to me better that the action that
preserved the user uses the option '--preserve' rather

It's ridiculous that the CLI taints the RPC API and it should be fixed.

Also on a more nitpicky side, I think the flag should be called
--no-preserve rather than --permanently. There is plenty of commands
(rm, cp, ...) which have --no-preserve as opposite of --preserve.

The attached patch fixes both.

... and it also accidentaly changes the default behavior.

Updated patch attached.

ACK if others are ok with changing --permanently to --no-preserve.

Patch 446 fixed also issue #5, patch 446.1 doesn't fix it. Could be fixed separately.

Attaching patch which addresses this API change in Web UI.
Petr Vobornik
From 0da4bd1487b83bfc24d42ffad66e2396e10ebb6a Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Thu, 18 Jun 2015 12:59:05 +0200
Subject: [PATCH] webui: adjust user deleter dialog to new api

In user_del, flags 'permanently' and 'preserve' were replaced with single
bool option 'preserve'

part of: https://fedorahosted.org/freeipa/ticket/3813
 install/ui/src/freeipa/user.js     | 22 ++++++++++------------
 install/ui/src/freeipa/widget.js   |  4 ++--
 install/ui/test/data/ipa_init.json |  3 +++
 ipalib/plugins/internal.py         |  3 +++
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/install/ui/src/freeipa/user.js b/install/ui/src/freeipa/user.js
index 09336c3cf2317ef5da6a355275cf5cf0748665a6..bf84ba3ac194e7ff7e172f70475a726ca6607a9c 100644
--- a/install/ui/src/freeipa/user.js
+++ b/install/ui/src/freeipa/user.js
@@ -833,13 +833,13 @@ IPA.user.deleter_dialog = function(spec) {
         that.option_radio = IPA.radio_widget({
-            name: 'deletemode',
-            label: 'Delete mode',
+            name: 'preserve',
+            label: '@i18n:objects.user.delete_mode',
             options: [
-                { label: 'default', value: '' },
-                { label: 'delete permanently', value: 'permanently' },
-                { label: 'preserve', value: 'preserve' }
-            ]
+                { label: '@i18n:objects.user.mode_delete', value: 'false' },
+                { label: '@i18n:objects.user.mode_preserve', value: 'true' }
+            ],
+            default_value: 'false'
         var html = that.option_layout.create([that.option_radio]);
@@ -849,13 +849,11 @@ IPA.user.deleter_dialog = function(spec) {
     that.create_command = function() {
         var batch = that.search_deleter_dialog_create_command();
-        var option = that.option_radio.get_value()[0];
+        var preserve = that.option_radio.get_value()[0];
-        if (option) {
-            for (var i=0; i<batch.commands.length; i++) {
-                var command = batch.commands[i];
-                command.set_option(option, true);
-            }
+        for (var i=0; i<batch.commands.length; i++) {
+            var command = batch.commands[i];
+            command.set_option('preserve', preserve);
         return batch;
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index e985cff227b39221da262b3a0c509c0e07ce606a..434a4b1bbe2ce1e71914f8543410de9212b389fe 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1425,8 +1425,8 @@ IPA.option_widget_base = function(spec, that) {
     // classic properties
     that.name = spec.name;
-    that.label = spec.label;
-    that.title = spec.title;
+    that.label = text.get(spec.label);
+    that.title = text.get(spec.title);
     that.sort = spec.sort === undefined ? false : spec.sort;
     that.value_changed = that.value_changed || IPA.observer();
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index e5d306cd0cc9d39e05ebce0c45538a859eb89992..1290db2c430354afdd79024dbda8752330d11aaf 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -578,11 +578,14 @@
                             "account_status": "Account Status",
                             "activeuser_label": "Active users",
                             "contact": "Contact Settings",
+                            "delete_mode": "Delete mode",
                             "employee": "Employee Information",
                             "error_changing_status": "Error changing account status",
                             "krbpasswordexpiration": "Password expiration",
                             "mailing": "Mailing Address",
                             "misc": "Misc. Information",
+                            "mode_delete": "delete",
+                            "mode_preserve": "preserve",
                             "noprivate": "No private group",
                             "status_confirmation": "Are you sure you want to ${action} the user?<br/>The change will take effect immediately.",
                             "status_link": "Click to ${action}",
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index ff096616d5e9bdc6737ee2d8b087fbded18ca2af..270a228b2713ac6b17cebb5f23158bc631d5e42d 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -724,11 +724,14 @@ class i18n_messages(Command):
                 "account_status": _("Account Status"),
                 "activeuser_label": _("Active users"),
                 "contact": _("Contact Settings"),
+                "delete_mode": _("Delete mode"),
                 "employee": _("Employee Information"),
                 "error_changing_status": _("Error changing account status"),
                 "krbpasswordexpiration": _("Password expiration"),
                 "mailing": _("Mailing Address"),
                 "misc": _("Misc. Information"),
+                "mode_delete": _("delete"),
+                "mode_preserve": _("preserve"),
                 "noprivate": _("No private group"),
                 "status_confirmation": _("Are you sure you want to ${action} the user?<br/>The change will take effect immediately."),
                 "status_link": _("Click to ${action}"),

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to