Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 5/6/2013 11:20 AM, Petr Vobornik wrote: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? I've mixed feelings about it. I would rather keep the methods simple - one task oriented. The issue was caused by other problem: The plugin had to wait for metadata and profile information (to check self-service). So the callback is called after menu is created and menu requires the entity to be resolved. Should we postpone the menu creation to a different phase? Maybe a new one? I'm not sure exactly how the menu works, but if you create another phase how will it address the problem? As a side note, if the phases are implemented as regular class methods, the plugins probably can just override the method. You won't need to introduce a new phase. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 05/03/2013 07:35 PM, Endi Sukma Dewata wrote: Hi, Sorry for the delay, I have some questions comments. Registry: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? Ideally a class should have complete methods to manage the objects it stores (e.g. unregister(), remove()). How is reg.entity created? Are there others beside 'entity'? How is Registries_registry in reg.js used? It doesn't seem to be used anywhere else. Plugins: In plugins.py the list of plugins is generated using os.listdir(). Then each plugin also has a list of dependencies which I suppose can include other plugins. Then when registering the plugin task, it will have a priority as well. So there seem to be several factors that determine the execution order of the plugins. There should be a document explaining how this will work, so plugin writers can be sure that the code will be executed at the right time. In general I'd avoid using task priority because it doesn't guarantee the correct execution order unless the priorities of all tasks are well coordinated (which might be challenging if there are multiple plugins owned by different people). Could you add more examples of simple plugins for various scenarios including custom entity, custom facet, custom field, custom menu? They can be included in the RPM for reference. Writing a plugin seems to still require programming skills, reliance on good docs, and probably even some source code familiarity. What do you think about simplifying this a little further? So we'll have 2 ways to define a plugin: one is programatically using the current framework already implemented (e.g. simpleuser.js), and the other is completely declaratively using a plain json data (e.g. simpleuser.json). The declarative plugin will obviously be more limited, but much simpler to use. Builder: b) Second big issue was build of objects. Entities and facets have complex build logic. It can be simplified into three steps: 1) modifications of spec 2) creation of object and class inheritance 3) init logic Yes, creating an object has become very complicated now with the builders, factories, constructors, preops, postops, inits, overrides, diff, etc. I think the problem is that we're trying to create/modify the spec before creating the object and we need a whole set of mechanisms to do that. Maybe we can simplify it into two basic steps: 1. Create an empty/simple object. 2. Initialize the object. The initialization process could be split further into smaller operations such as: * Load the spec and modify it if necessary * Creating dependent objects and initializing them * Other initialization steps The builder, factory, preops, and postops can be included as part of the initialization step. They can be normal class methods rather than loosely defined functions and can be overridden by subclasses. There's probably a lot more details that need to be discussed. 1. Move ./_base/metadata_provider to ./metadata? Might simplify stuff. This seems to be IPA-specific, so yes. 2. Move actions/buttons spec from factories to pre_ops associated with the factories. Example of stuff to be moved (search.js): spec.actions = spec.actions || []; spec.actions.unshift( 'refresh', 'batch_remove', 'add'); It may simplify/allow removal of those items in spec or pre_ops of child factories. Currently there is no way how to intercept them. Sure, I don't see any problem with that. In general there is no major issue that would warrant a NACK. As long as the API is well documented for plugin writers it should be sufficient. Thanks for review Endi! Since we do not have a NACK, lets do small changes we can do now for 3.2 GA and create tickets for the rest (can be done in 3.2 stabilization phase). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 05/03/2013 07:35 PM, Endi Sukma Dewata wrote: Hi, Sorry for the delay, I have some questions comments. Registry: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? I've mixed feelings about it. I would rather keep the methods simple - one task oriented. The issue was caused by other problem: The plugin had to wait for metadata and profile information (to check self-service). So the callback is called after menu is created and menu requires the entity to be resolved. Should we postpone the menu creation to a different phase? Maybe a new one? Ideally a class should have complete methods to manage the objects it stores (e.g. unregister(), remove()). This can be added. How is reg.entity created? Are there others beside 'entity'? Registries are created in related modules. So reg.entity get instantiated at the end of entity.js. There is one for each object type, which are, IIRC: action, facet, entity, field, validator and some others. How is Registries_registry in reg.js used? It doesn't seem to be used anywhere else. Wow, I forgot about this peace of code. It isn't and should be removed. Plugins: In plugins.py the list of plugins is generated using os.listdir(). Then each plugin also has a list of dependencies which I suppose can include other plugins. Then when registering the plugin task, it will have a priority as well. So there seem to be several factors that determine the execution order of the plugins. There should be a document explaining how this will work, so plugin writers can be sure that the code will be executed at the right time. In general I'd avoid using task priority because it doesn't guarantee the correct execution order unless the priorities of all tasks are well coordinated (which might be challenging if there are multiple plugins owned by different people). You are right and it gets more complicated with asynchronous tasks. If one wants to react to an asynchronous task, he has to do it next phase. We might set some fixed priorities for certain operations. Anyway the documentation is essential. Could you add more examples of simple plugins for various scenarios including custom entity, custom facet, custom field, custom menu? They can be included in the RPM for reference. Yes, I that's my plan. Another reason to do it to find limitations. Writing a plugin seems to still require programming skills, reliance on good docs, and probably even some source code familiarity. What do you think about simplifying this a little further? So we'll have 2 ways to define a plugin: one is programatically using the current framework already implemented (e.g. simpleuser.js), and the other is completely declaratively using a plain json data (e.g. simpleuser.json). The declarative plugin will obviously be more limited, but much simpler to use. I agree with the idea. But before creating declarative JSON format we should come up with a plugin API, which would be more-or-less stable and therefore plugins might be resilient to Web UI internals changes. When we have this API we might map it to a JSON representation. The hard part will be to find all the use-cases to cover. Builder: b) Second big issue was build of objects. Entities and facets have complex build logic. It can be simplified into three steps: 1) modifications of spec 2) creation of object and class inheritance 3) init logic Yes, creating an object has become very complicated now with the builders, factories, constructors, preops, postops, inits, overrides, diff, etc. I think the problem is that we're trying to create/modify the spec before creating the object and we need a whole set of mechanisms to do that. Maybe we can simplify it into two basic steps: 1. Create an empty/simple object. 2. Initialize the object. The initialization process could be split further into smaller operations such as: * Load the spec and modify it if necessary * Creating dependent objects and initializing them * Other initialization steps The builder, factory, preops, and postops can be included as part of the initialization step. They can be normal class methods rather than loosely defined functions and can be overridden by subclasses. There's probably a lot more details that need to be discussed. +1 Rewriting all the factories into classes will be a huge task though. At the moment, the biggest problem are spec modification which are not defaults (so they can't be overriden). Like the ones described bellow - #2. 1. Move ./_base/metadata_provider to ./metadata? Might simplify stuff. This seems to be IPA-specific, so yes. https://fedorahosted.org/freeipa/ticket/3604 2. Move actions/buttons spec from factories to pre_ops associated with the factories. Example of stuff to be moved (search.js):
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 04/26/2013 12:51 PM, Petr Vobornik wrote: On 04/25/2013 06:37 PM, Ana Krivokapic wrote: 1) When in self service mode, you are now allowed to go to pages of related objects. If you go to e.g. User Groups for your user, there are Add/Delete buttons and they are enabled, but if you try to use them, you will be denied access. However, a message will appear, saying 'Items added' / 'Items removed' even though the operation had failed. Should we disable these options in self service mode? I think we should at least make sure that the misleading message which suggests that the actions was completed, does not appear. Regression. Links should not be clickable and buttons should be hidden. Add/Delete buttons are still enabled in self-service mode (e.g. https://ipahost/ipa/ui/index.html#/e/group/member_user/admins). I have found no other issues in recent commits (apart from the ones already reported). -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 05/03/2013 12:44 PM, Ana Krivokapic wrote: On 04/26/2013 12:51 PM, Petr Vobornik wrote: On 04/25/2013 06:37 PM, Ana Krivokapic wrote: 1) When in self service mode, you are now allowed to go to pages of related objects. If you go to e.g. User Groups for your user, there are Add/Delete buttons and they are enabled, but if you try to use them, you will be denied access. However, a message will appear, saying 'Items added' / 'Items removed' even though the operation had failed. Should we disable these options in self service mode? I think we should at least make sure that the misleading message which suggests that the actions was completed, does not appear. Regression. Links should not be clickable and buttons should be hidden. Add/Delete buttons are still enabled in self-service mode (e.g. https://ipahost/ipa/ui/index.html#/e/group/member_user/admins). I have found no other issues in recent commits (apart from the ones already reported). I don't think that this is an issue. The buttons are disabled on self-service page. Other pages, accessible only by manual change of URL, aren't important in self-service mode. Question is if we should limit navigation only to user pages or just user details page. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
Update: * added ticket number to every commit, some commit messages improved * ~4 commits squashed * rebased on current master On 04/30/2013 07:19 PM, Petr Vobornik wrote: Update: * rebased on current master an force-pushed to private repo * fixed crash when IPA installed without CA * fixed bugs found by automated tests ** crash on ssh key add ** crash on host deletion * added design page for #3236: http://www.freeipa.org/page/V3/WebUI_extensible_navigation * enabled adding facets without entity into menu (untested) Attaching two simple example plugins. * simpleuser: replaces selfservice page with simpler one * usermod: moves initials field in user page ** example of adding a field ** example of deleting a field ** tries Spec_mod utility which proved to not be much useful yet because it's search is limited only to one array, which is not enough To test the plugins put them into: /usr/share/ipa/ui/js/plugins/usermod /usr/share/ipa/ui/js/plugins/simpleuser On 04/26/2013 06:15 PM, Petr Vobornik wrote: Another problem found: trustconfig had invalid spec which made the trust section quite unusable. Fixed and pushed to the private repo. I also took an opportunity and added missing parts of trust metadata for static testing. On 04/26/2013 04:32 PM, Petr Vobornik wrote: Hi, 1, 2 and 3a are fixed and pushed to the private repo. Rest won't be fixed during the refactoring. I changed SingletonRegistry behavior that it returns null when builder/construction spec is missing. It's more consistent with build of unsupported entities (also returns null). -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
Hi, Sorry for the delay, I have some questions comments. Registry: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? Ideally a class should have complete methods to manage the objects it stores (e.g. unregister(), remove()). How is reg.entity created? Are there others beside 'entity'? How is Registries_registry in reg.js used? It doesn't seem to be used anywhere else. Plugins: In plugins.py the list of plugins is generated using os.listdir(). Then each plugin also has a list of dependencies which I suppose can include other plugins. Then when registering the plugin task, it will have a priority as well. So there seem to be several factors that determine the execution order of the plugins. There should be a document explaining how this will work, so plugin writers can be sure that the code will be executed at the right time. In general I'd avoid using task priority because it doesn't guarantee the correct execution order unless the priorities of all tasks are well coordinated (which might be challenging if there are multiple plugins owned by different people). Could you add more examples of simple plugins for various scenarios including custom entity, custom facet, custom field, custom menu? They can be included in the RPM for reference. Writing a plugin seems to still require programming skills, reliance on good docs, and probably even some source code familiarity. What do you think about simplifying this a little further? So we'll have 2 ways to define a plugin: one is programatically using the current framework already implemented (e.g. simpleuser.js), and the other is completely declaratively using a plain json data (e.g. simpleuser.json). The declarative plugin will obviously be more limited, but much simpler to use. Builder: b) Second big issue was build of objects. Entities and facets have complex build logic. It can be simplified into three steps: 1) modifications of spec 2) creation of object and class inheritance 3) init logic Yes, creating an object has become very complicated now with the builders, factories, constructors, preops, postops, inits, overrides, diff, etc. I think the problem is that we're trying to create/modify the spec before creating the object and we need a whole set of mechanisms to do that. Maybe we can simplify it into two basic steps: 1. Create an empty/simple object. 2. Initialize the object. The initialization process could be split further into smaller operations such as: * Load the spec and modify it if necessary * Creating dependent objects and initializing them * Other initialization steps The builder, factory, preops, and postops can be included as part of the initialization step. They can be normal class methods rather than loosely defined functions and can be overridden by subclasses. There's probably a lot more details that need to be discussed. 1. Move ./_base/metadata_provider to ./metadata? Might simplify stuff. This seems to be IPA-specific, so yes. 2. Move actions/buttons spec from factories to pre_ops associated with the factories. Example of stuff to be moved (search.js): spec.actions = spec.actions || []; spec.actions.unshift( 'refresh', 'batch_remove', 'add'); It may simplify/allow removal of those items in spec or pre_ops of child factories. Currently there is no way how to intercept them. Sure, I don't see any problem with that. In general there is no major issue that would warrant a NACK. As long as the API is well documented for plugin writers it should be sufficient. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
Update: * rebased on current master an force-pushed to private repo * fixed crash when IPA installed without CA * fixed bugs found by automated tests ** crash on ssh key add ** crash on host deletion * added design page for #3236: http://www.freeipa.org/page/V3/WebUI_extensible_navigation * enabled adding facets without entity into menu (untested) Attaching two simple example plugins. * simpleuser: replaces selfservice page with simpler one * usermod: moves initials field in user page ** example of adding a field ** example of deleting a field ** tries Spec_mod utility which proved to not be much useful yet because it's search is limited only to one array, which is not enough To test the plugins put them into: /usr/share/ipa/ui/js/plugins/usermod /usr/share/ipa/ui/js/plugins/simpleuser On 04/26/2013 06:15 PM, Petr Vobornik wrote: Another problem found: trustconfig had invalid spec which made the trust section quite unusable. Fixed and pushed to the private repo. I also took an opportunity and added missing parts of trust metadata for static testing. On 04/26/2013 04:32 PM, Petr Vobornik wrote: Hi, 1, 2 and 3a are fixed and pushed to the private repo. Rest won't be fixed during the refactoring. I changed SingletonRegistry behavior that it returns null when builder/construction spec is missing. It's more consistent with build of unsupported entities (also returns null). -- Petr Vobornik simpleuser.js Description: application/javascript usermod.js Description: application/javascript ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 04/25/2013 06:37 PM, Ana Krivokapic wrote: Hi, While reviewing and testing the new UI changes, I have encountered the following issues. (Some of them may be unrelated to the webUI refactoring effort, but I will list them here just so we are aware of them.) Thanks for review, I will fix regressions today. We might open a ticket for the rest. 1) When in self service mode, you are now allowed to go to pages of related objects. If you go to e.g. User Groups for your user, there are Add/Delete buttons and they are enabled, but if you try to use them, you will be denied access. However, a message will appear, saying 'Items added' / 'Items removed' even though the operation had failed. Should we disable these options in self service mode? I think we should at least make sure that the misleading message which suggests that the actions was completed, does not appear. Regression. Links should not be clickable and buttons should be hidden. 2) This one was already discussed with Petr in person: Runtime error on invalid URL: https://ipahost/ipa/ui/#/e/doesnotexist will give an ugly runtime error and any further navigation does not get rid of this error - you have to reload the page. We should make sure that this is handled more gracefully. Kind of regression. I will fix it by redirecting to default facet (user search). 3) Role Based Access Control, when trying to add a permission to a privilege: * Permissions which are already in that privilege appear in the list of available permissions. They should not appear there (it doesn't make sense to add something which is already there). (This behavior is correct in other parts of UI, e.g. when you want add a privilege to a role, the privileges which are already present for that role, do not appear in the list of available privileges.) * When you try to add such permission, first an Operations Error appears, but when you click OK, a message saying 'Items added' appears (similar issue is mentioned in 1) ). Not related to refactoring. a) permission-find doesn't have not-in-priviledge options so the not-offering of existing values is handled by association_adder_dialog dialog's `exclude` array. This array is filled with already added and then compared with the keys got from xx-find command. Problems is that memberships are lowercased and therefore the values have different casing - don't match and therefore are still offered. We might do some normalization in association_adder_dialog. I don't mind fixing it along with the refactoring. b) the success message might be a more wide-spread problem I noticed similar behavior in group's external member. Because of the wider scope please open a ticket. 4) Host Based Access Control: When modifying a HBAC rule, workflow can be a bit confusing. For example, if you have a rule with 'Anyone' selected in the 'WHO' section, then you decide to change it to Specified Users and Groups, and then click on Add to add users/groups, a dialog appears requiring you to save your selection first (you have to click on Update, or click Cancel, then Update the changes and then try to Add users again). Is it possible to call the Update when Add is clicked, so that this step is automatically performed, requiring no action from the user? I think it would feel more intuitive to the user. I see one problem with the proposal. User might have changed also a description field or other category rule radio. I'm not sure if he wants to apply these changes automatically as well. 5) For sections that have Expand All/Collapse All link (for example, when looking at a user's details page), I think that when you expand all sections manually, the Expand All link should change to Collapse all. And also the other way around: when you collapse all sections manually, the Collapse All link should change to Expand All. This is probably nitpicking too much, it is just a nice to have (does not make sense to 'expand all' if everything is already expanded). Open a ticket. I wonder how many users is using this feature. Personally, I don't. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
Hi, 1, 2 and 3a are fixed and pushed to the private repo. Rest won't be fixed during the refactoring. I changed SingletonRegistry behavior that it returns null when builder/construction spec is missing. It's more consistent with build of unsupported entities (also returns null). On 04/26/2013 12:51 PM, Petr Vobornik wrote: On 04/25/2013 06:37 PM, Ana Krivokapic wrote: Hi, While reviewing and testing the new UI changes, I have encountered the following issues. (Some of them may be unrelated to the webUI refactoring effort, but I will list them here just so we are aware of them.) Thanks for review, I will fix regressions today. We might open a ticket for the rest. 1) When in self service mode, you are now allowed to go to pages of related objects. If you go to e.g. User Groups for your user, there are Add/Delete buttons and they are enabled, but if you try to use them, you will be denied access. However, a message will appear, saying 'Items added' / 'Items removed' even though the operation had failed. Should we disable these options in self service mode? I think we should at least make sure that the misleading message which suggests that the actions was completed, does not appear. Regression. Links should not be clickable and buttons should be hidden. 2) This one was already discussed with Petr in person: Runtime error on invalid URL: https://ipahost/ipa/ui/#/e/doesnotexist will give an ugly runtime error and any further navigation does not get rid of this error - you have to reload the page. We should make sure that this is handled more gracefully. Kind of regression. I will fix it by redirecting to default facet (user search). 3) Role Based Access Control, when trying to add a permission to a privilege: * Permissions which are already in that privilege appear in the list of available permissions. They should not appear there (it doesn't make sense to add something which is already there). (This behavior is correct in other parts of UI, e.g. when you want add a privilege to a role, the privileges which are already present for that role, do not appear in the list of available privileges.) * When you try to add such permission, first an Operations Error appears, but when you click OK, a message saying 'Items added' appears (similar issue is mentioned in 1) ). Not related to refactoring. a) permission-find doesn't have not-in-priviledge options so the not-offering of existing values is handled by association_adder_dialog dialog's `exclude` array. This array is filled with already added and then compared with the keys got from xx-find command. Problems is that memberships are lowercased and therefore the values have different casing - don't match and therefore are still offered. We might do some normalization in association_adder_dialog. I don't mind fixing it along with the refactoring. b) the success message might be a more wide-spread problem I noticed similar behavior in group's external member. Because of the wider scope please open a ticket. 4) Host Based Access Control: When modifying a HBAC rule, workflow can be a bit confusing. For example, if you have a rule with 'Anyone' selected in the 'WHO' section, then you decide to change it to Specified Users and Groups, and then click on Add to add users/groups, a dialog appears requiring you to save your selection first (you have to click on Update, or click Cancel, then Update the changes and then try to Add users again). Is it possible to call the Update when Add is clicked, so that this step is automatically performed, requiring no action from the user? I think it would feel more intuitive to the user. I see one problem with the proposal. User might have changed also a description field or other category rule radio. I'm not sure if he wants to apply these changes automatically as well. 5) For sections that have Expand All/Collapse All link (for example, when looking at a user's details page), I think that when you expand all sections manually, the Expand All link should change to Collapse all. And also the other way around: when you collapse all sections manually, the Collapse All link should change to Expand All. This is probably nitpicking too much, it is just a nice to have (does not make sense to 'expand all' if everything is already expanded). Open a ticket. I wonder how many users is using this feature. Personally, I don't. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
Another problem found: trustconfig had invalid spec which made the trust section quite unusable. Fixed and pushed to the private repo. I also took an opportunity and added missing parts of trust metadata for static testing. On 04/26/2013 04:32 PM, Petr Vobornik wrote: Hi, 1, 2 and 3a are fixed and pushed to the private repo. Rest won't be fixed during the refactoring. I changed SingletonRegistry behavior that it returns null when builder/construction spec is missing. It's more consistent with build of unsupported entities (also returns null). -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 04/24/2013 04:55 PM, Petr Vobornik wrote: I've implemented the remaining work. Pushed to the private repo. Know problems remaining work -- 1. Change generation of plugin index to dynamical instead of rpm-post The plugin index (plugins.js) is generated by wsgi script. New dir was created: /usr/share/ipa/wsgi to store the script. It has the same attributes as migration dir. Plugins.js should be located in /usr/share/ipa/ui/js/freeipa/ dir. New rewrite rule was added in order to make it work. It has a nice side effect that one could not find out that the file is dynamically generated. Design page updated accordingly: http://www.freeipa.org/page/V3/WebUI_plugins 2. Incorrect behavior (enabled buttons) of rule table when 'rule applies to anyone' selected. Fixed by creating updated event. Probably not caused by this refactoring but by refactoring of checkboxes and radios for PAC patch. 3. delete ./facets module Use ./reg an ./builder instead. Incorporate it into router to support standalone facets. Done, but not tested. I'll create plugin example to test it. Hi, While reviewing and testing the new UI changes, I have encountered the following issues. (Some of them may be unrelated to the webUI refactoring effort, but I will list them here just so we are aware of them.) 1) When in self service mode, you are now allowed to go to pages of related objects. If you go to e.g. User Groups for your user, there are Add/Delete buttons and they are enabled, but if you try to use them, you will be denied access. However, a message will appear, saying 'Items added' / 'Items removed' even though the operation had failed. Should we disable these options in self service mode? I think we should at least make sure that the misleading message which suggests that the actions was completed, does not appear. 2) This one was already discussed with Petr in person: Runtime error on invalid URL: https://ipahost/ipa/ui/#/e/doesnotexist will give an ugly runtime error and any further navigation does not get rid of this error - you have to reload the page. We should make sure that this is handled more gracefully. 3) Role Based Access Control, when trying to add a permission to a privilege: * Permissions which are already in that privilege appear in the list of available permissions. They should not appear there (it doesn't make sense to add something which is already there). (This behavior is correct in other parts of UI, e.g. when you want add a privilege to a role, the privileges which are already present for that role, do not appear in the list of available privileges.) * When you try to add such permission, first an Operations Error appears, but when you click OK, a message saying 'Items added' appears (similar issue is mentioned in 1) ). 4) Host Based Access Control: When modifying a HBAC rule, workflow can be a bit confusing. For example, if you have a rule with 'Anyone' selected in the 'WHO' section, then you decide to change it to Specified Users and Groups, and then click on Add to add users/groups, a dialog appears requiring you to save your selection first (you have to click on Update, or click Cancel, then Update the changes and then try to Add users again). Is it possible to call the Update when Add is clicked, so that this step is automatically performed, requiring no action from the user? I think it would feel more intuitive to the user. 5) For sections that have Expand All/Collapse All link (for example, when looking at a user's details page), I think that when you expand all sections manually, the Expand All link should change to Collapse all. And also the other way around: when you collapse all sections manually, the Collapse All link should change to Expand All. This is probably nitpicking too much, it is just a nice to have (does not make sense to 'expand all' if everything is already expanded). -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
I've implemented the remaining work. Pushed to the private repo. Know problems remaining work -- 1. Change generation of plugin index to dynamical instead of rpm-post The plugin index (plugins.js) is generated by wsgi script. New dir was created: /usr/share/ipa/wsgi to store the script. It has the same attributes as migration dir. Plugins.js should be located in /usr/share/ipa/ui/js/freeipa/ dir. New rewrite rule was added in order to make it work. It has a nice side effect that one could not find out that the file is dynamically generated. Design page updated accordingly: http://www.freeipa.org/page/V3/WebUI_plugins 2. Incorrect behavior (enabled buttons) of rule table when 'rule applies to anyone' selected. Fixed by creating updated event. Probably not caused by this refactoring but by refactoring of checkboxes and radios for PAC patch. 3. delete ./facets module Use ./reg an ./builder instead. Incorporate it into router to support standalone facets. Done, but not tested. I'll create plugin example to test it. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On Wed, 24 Apr 2013, Petr Vobornik wrote: I've implemented the remaining work. Pushed to the private repo. Know problems remaining work -- 1. Change generation of plugin index to dynamical instead of rpm-post The plugin index (plugins.js) is generated by wsgi script. New dir was created: /usr/share/ipa/wsgi to store the script. It has the same attributes as migration dir. Plugins.js should be located in /usr/share/ipa/ui/js/freeipa/ dir. New rewrite rule was added in order to make it work. It has a nice side effect that one could not find out that the file is dynamically generated. 1. We should not elevate privileges to wsgi script. Instead, one could do plugin list regeneration by running pre-start script in ipa systemd unit. Alternatively, we can add ipa-js-plugins.service unit that is run one-off and is required by ipa.service. 2. /usr/share/ipa/wsgi is wrong. In long term Fedora is moving to make /usr/share read-only. I'd rather moved it to /var/cache/ipa/wsgi. wsgi process already knows how to reach to /var/cache/ipa/sessions so we are good from SELinux perspective as well. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 04/24/2013 06:03 PM, Alexander Bokovoy wrote: On Wed, 24 Apr 2013, Petr Vobornik wrote: I've implemented the remaining work. Pushed to the private repo. Know problems remaining work -- 1. Change generation of plugin index to dynamical instead of rpm-post The plugin index (plugins.js) is generated by wsgi script. New dir was created: /usr/share/ipa/wsgi to store the script. It has the same attributes as migration dir. Plugins.js should be located in /usr/share/ipa/ui/js/freeipa/ dir. New rewrite rule was added in order to make it work. It has a nice side effect that one could not find out that the file is dynamically generated. 1. We should not elevate privileges to wsgi script. Instead, one could do plugin list regeneration by running pre-start script in ipa systemd unit. Alternatively, we can add ipa-js-plugins.service unit that is run one-off and is required by ipa.service. 2. /usr/share/ipa/wsgi is wrong. In long term Fedora is moving to make /usr/share read-only. I'd rather moved it to /var/cache/ipa/wsgi. wsgi process already knows how to reach to /var/cache/ipa/sessions so we are good from SELinux perspective as well. The wsgi script doesn't write anything. It just reads a content of /usr/share/ipa/ui/js/plugins directory, transforms it into JS AMD module with one array and returns it as an application/javascript http response. My inspiration was /ipa/migration/migration.py. The difference is that plugins.py reads dir and migration.py communicates with LDAP through ipalib. Is the reading of dir content also problematic? -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On Wed, 24 Apr 2013, Petr Vobornik wrote: On 04/24/2013 06:03 PM, Alexander Bokovoy wrote: On Wed, 24 Apr 2013, Petr Vobornik wrote: I've implemented the remaining work. Pushed to the private repo. Know problems remaining work -- 1. Change generation of plugin index to dynamical instead of rpm-post The plugin index (plugins.js) is generated by wsgi script. New dir was created: /usr/share/ipa/wsgi to store the script. It has the same attributes as migration dir. Plugins.js should be located in /usr/share/ipa/ui/js/freeipa/ dir. New rewrite rule was added in order to make it work. It has a nice side effect that one could not find out that the file is dynamically generated. 1. We should not elevate privileges to wsgi script. Instead, one could do plugin list regeneration by running pre-start script in ipa systemd unit. Alternatively, we can add ipa-js-plugins.service unit that is run one-off and is required by ipa.service. 2. /usr/share/ipa/wsgi is wrong. In long term Fedora is moving to make /usr/share read-only. I'd rather moved it to /var/cache/ipa/wsgi. wsgi process already knows how to reach to /var/cache/ipa/sessions so we are good from SELinux perspective as well. The wsgi script doesn't write anything. It just reads a content of /usr/share/ipa/ui/js/plugins directory, transforms it into JS AMD module with one array and returns it as an application/javascript http response. Sorry, I've missed this part when looking through the code. This approach is good then -- we don't modify anything on the file system, only read. My inspiration was /ipa/migration/migration.py. The difference is that plugins.py reads dir and migration.py communicates with LDAP through ipalib. Is the reading of dir content also problematic? That is completely fine -- after all serving those .js files implies httpd_context_t being able to read them. Sorry for false alarm! -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
