[Freeipa-devel] [PATCH] 107 Content is no more overwritten by error message

[Endi Sukma Dewata]

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