On 3/9/2012 7:16 AM, Petr Vobornik wrote:
When an error which caused calling of report_error occur, the content of
a facet got replaced by error message. There was no way how to force the
facet to recreate its content and the facet became unusable.
This patch creates a container for an error message. On error,
report_error writes its content to error container, content container is
hidden and error container is shown. Older comment in a code suggested
to move the error message to facet's footer. A message in a footer could
be missed by the user and on top of that a footer is sometimes used by
various facet and we would have to solve the same problem again.
From experience the cause of an error is usually a missing pkey in a
path. Therefore error information suggests user to navigate to top
level. It causes to load default facets with default values so errors in
navigation state shouldn't happen.
Facet content is displayed back on facet_show. If user tries to display
same object as before facet's need_update() would return false,
therefore need_update was modified to always return true if error is
displayed.
Also it may be possible to display facet content on refresh success. I
tried to avoid that because it would require putting show_content calls
to various success handlers or load methods. It would add one more thing
developer needs to think of when overriding refresh or load method.
Reproduction:
1) display any nested entity - ie DNS record
2) delete its parent pkey from path - &dnszone-pkey=example.com
3) reload the page with this path
https://fedorahosted.org/freeipa/ticket/2449
In the ticket I added 2 more scenarios to reproduce the problem. So we
have 3 possible cases:
1. invalid UI state
2. non-existent entry
3. server down
For case #1, the patch provides a much better message, but I think
ideally if some parameters are missing from the URL (e.g. the parent
pkey) it should be detected by the UI before sending the request to the
server. This probably should be addressed in a separate ticket. See the
note below about the error message.
For case #2, the patch fixes the issue by clearing up the error message.
This works on all entities except users because the user details page
uses a batch operation to get the data and it doesn't handle
non-existent users properly. I think this is an existing and separate
issue.
For case #3, the patch will show a message saying that the UI got into
an invalid state, which is actually not the case here. Also, returning
to the main page or reloading the page wouldn't solve the problem either.
So for this ticket I think it would be better to show a more generic
error message, something like this:
An error has occured (IPA Error 3007)
'idnsname' is required
Please try the following options:
* Refresh the page. (see note below)
* Return to the main page and retry the operation.
* Reload the browser.
If the problem persists please contact the system administrator.
Each of the above options could be made into a link that does the
mentioned operation.
It would be great if we can use the Refresh button to clear the error
message. If this requires significant effort we probably can remove this
option from the message above and add it in a separate ticket.
One more thing, this may not be a problem now, but the error_container
uses both facet-content and facet-error CSS classes. I understand this
is done to avoid code duplication, but this also means the facet will
have 2 facet-contents. CSS classes can be used for decorative or
structural purposes or both, so we need to make sure decorative changes
will not affect it structurally. One solution is to duplicate the CSS
code from facet-content into facet-error. Another solution is to use a
separate decorative class that are added into both facet-content and
facet-error elements.
--
Endi S. Dewata
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
