Affects Version/s: None
Fix Version/s: Not version specific
Security Level: Public (Public: Anyone can view)
KULRICE-5323 Dataset Consolidation KULRICE-111 Simplify UI control pluggability KULRICE-1306 Data/DDL consolidation and bootstrap process KULRICE-6321 Database script consolidation for release KULRICE-10878 Simplify ObjectPropertyUtils KULRICE-11597 Generated lookup/inquiry ids contain invalid characters KULRICE-13283 Grouping classes and ids bad for row grouping in collection KULRICE-8424 Consolidate database scripts for release KULRICE-2177 Consolidate all Rice Struts modules into a single struts module KULRICE-7871 Checkbox and Radio Group ids are not correct
KRAD Feature Area:UIF Component
KAI Review Status:Not Required
KTI Review Status:Not Required
Code Review Status:Not Required
Include in Release Notes?:Yes
Cleanup item moved from
Extract ID assignment code (org.kuali.rice.krad.uif.service.impl.ViewHelperServiceImpl#preprocessView) to helper method or class, along with Component State, Create helper methods to reduce nesting and duplicate code
Right. So we could change AssignIdsTasks to use the same code as ViewHelperServiceImpl#preprocessView, that is using the path instead of the index?
I think the concern with testing, even after we get the things stable, is if a developer makes a change that moves the component (say push it down in a list or something of that sort), the scripts would need to be modified. If we had a way of generating the Ids (for example based on name for inputs), that would allow for more stable tests.
From: Mark Fyffe firstname.lastname@example.org
Sent: Thursday, December 12, 2013 8:22 AM
To: 'Jerry Neal'
Subject: RE: ids
This does make sense. Now that path is available by virtue of getComponentsForLifecycle(), it should be feasible to universally assign ID based on path.
There are several cases where components are dynamically created and added to the lifecycle, particularly during the apply model phase. Although it doesn’t do so right now, spawnSubLifecycle can assign the path in these cases making path a viable seed for generating the IDs even for new components. This won’t help the generateAndAssignIds() method used by LightTable and BreadcrumbOptions, however, since the purpose of this method appears to be to discard existing and assign new IDs to all components in the subtree – do you know what purpose this serves?
One reason IDs keep changing is because we are making changes to when they are assigned, and also changes that affect the order in which lifecycle components are created. Once the development effort settles the IDs should stabilize – however, I’d expect that the upcoming focus on DOM and component cleanup will continue to shake up recorded scripts.
From: Jerry Neal email@example.com
Sent: Wednesday, December 11, 2013 2:16 PM
To: 'Mark Fyffe'
Subject: RE: ids
Thanks for the information! I found the problem. It is due to the way I am running the lifecycle now for component refresh. The index on the phase was not the same and therefore I was getting different ids.
I do think it would be good to consolidate the ID code. In fact, we might want to add the method to Component. We have been discussing IDs recently due to complaints from testers. The issue is, the IDs we are creating could change if a developer modifies the structure. Recorded scripts will grab these ids (if present), and then not work after updates. For things like inputs, it would be better if we didn’t have the Id on there, and selenium would use the name. However, we need the id in many cases. So we have been talking about how we can make these more static across updates. One approach is to use information from the component. For example, data/input fields have the property path which should be unique in most cases. Actions have the method to call, etc. So we might need different Id generation methods depending on the component. Does that make sense?
From: Mark Fyffe firstname.lastname@example.org
Sent: Tuesday, December 10, 2013 3:43 PM
To: Jerry Neal
Subject: Re: ids
There are three places now that generate IDs - each will produce a different ID but the results should be repeatable given the same set of circumstances. It will probably be best to consolidate these to a single utility - although it is not currently the case, path is available during the lifecycle, and within pre-process phase, so it should be possible to base all IDs on the path.
ViewHelperServiceImpl.preprocessView() - Assigns IDs to all components involved in the initialize phase prior to caching the view. Uses path, class name, and parent component ids to maintain an evolving hash code as it traverses the tree. The results should be the same each time the same view is loaded and cached from the DD.
AssignIdsTask - Assigns IDs during the initialize phase based on the class name and list position for all predecessor phases in the lifecycle phase tree. The results should be the same on each run as long as the components are processed in the same order. IDs are only assigned if not already present. This covers components dynamically created and spawned.
ComponentUtils.generateAndAssignIds() - Used by LightTable and BreadcrumbOptions. IDs use an evolving hash based on class name and previously assigned ID. Current IDs are discarded and replaced with new values. Results should be repeatable as long as the components provided are consistent.
In all cases, the ID generated is checked against a set maintained in ViewIndex to prevent duplicate IDs from being generated for the same view. If the ID has already been generated, it is considered a hash collision, so will be seeded again until an ID that hasn't been used for the current view is generated.
Off the top of my head, I can't think of a reason why a component would get a new ID on refresh. I does seem possible, though, since ID assignment and manipulation takes place during performComponentLifecycle(). Do you have an example of where it is happening?
On Tue, Dec 10, 2013 at 2:45 PM, Jerry Neal <email@example.com> wrote:
The new Id generation strategy you put in is based on the component path correct? So if we refreshed a component, the id that gets generated should be the same correct? For some reason, I am not seeing this happen currently.