Skip to content

Fix error messages on failed deletes, updates#1130

Closed
RichardBradley wants to merge 1 commit intomarmelab:masterfrom
RichardBradley:error_message_on_failed_delete
Closed

Fix error messages on failed deletes, updates#1130
RichardBradley wants to merge 1 commit intomarmelab:masterfrom
RichardBradley:error_message_on_failed_delete

Conversation

@RichardBradley
Copy link
Copy Markdown
Contributor

@RichardBradley RichardBradley commented Jun 16, 2016

This is a fix for items 1 & 2 described at #1129 (comment)

It does not fix item 3.

New test results:

Given an entity where the REST backend will return 400 "cannot delete due to xyz" for a "DELETE" request
When I view the "edit" page for that entity
And I click the "delete" button
And I click "Yes" to "are you sure?"
Then I should see an error banner including "cannot delete due to xyz"
 # this passes now (and fails before these changes)

Given an entity where the REST backend will return 400 "cannot delete due to xyz" for a "DELETE" request
When I view the "list" page for that entity tytpe
And I select that entity
And I click the "delete" button
And I click "Yes" to "are you sure?"
Then I should see an error banner including "cannot delete due to xyz"
 # this still fails due to item #3
 # I see a success banner

This was referenced Jun 17, 2016
Open
Open
@jpetitcolas jpetitcolas added this to the 1.0 milestone Sep 19, 2016
@jpetitcolas
Copy link
Copy Markdown
Contributor

jpetitcolas commented Sep 19, 2016

Thanks for your PR. Just rebasing it and fixing tests in a new one to be able to merge it today (we plan to release a new alpha later today). :)

@jpetitcolas jpetitcolas mentioned this pull request Sep 19, 2016
Merged
@fzaninotto fzaninotto changed the title #1129 Fix error messages on failed deletes, updates Fix error messages on failed deletes, updates Dec 1, 2016
@Phocea
Copy link
Copy Markdown
Contributor

Phocea commented Dec 10, 2016

Hey guys. I was just think how close this error handling is from the [PR] (#1273) I submitted.
PR is for http error on state change. This one is affecting handling of HTTP error on CRUD operations.

Cant we think of a common way to handle this rather than to have 2 different methods?

  • State change can now be customised using a decorator
  • CRUD errors can be customised using the entity.errorMessage(). I have not tried yet, but couldnt we have errorMessage use the HttpErrorService by default now ?

@jpetitcolas
Copy link
Copy Markdown
Contributor

jpetitcolas commented Dec 20, 2016

It seems these changes are already on master. Closing the issue.

@Phocea: indeed, it sounds like a good idea, yet let's apply it later. Short term objective is to release 1.0 version. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@RichardBradley @jpetitcolas @Phocea