Defect #11691

404 response when deleting a user from the edit page

Added by Etienne Massip about 5 years ago. Updated about 5 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Accounts / authentication
Target version:2.1.0
Resolution:Fixed Affected version:1.4.2

Description

Since r9678 for #10865, deleting a user from the user edit page redirects to the deleted user edit page, a.k.a HTTP 404 error page.


Related issues

Related to Redmine - Defect #10865: Filter reset when deleting locked user Closed

Associated revisions

Revision 10234
Added by Etienne Massip about 5 years ago

Use a back_url parameter instead of referrer to refresh the page after user deletion (#11691).

Revision 10235
Added by Etienne Massip about 5 years ago

Reverted changes made to tests for r1893 (#11691).

Revision 10238
Added by Jean-Philippe Lang about 5 years ago

Reverted r10234 and r10235 that broke redirect after login (#11691).

Tests in account_controller_test.rb should not have been changed.

Revision 10239
Added by Jean-Philippe Lang about 5 years ago

Do not use escaped back_url param (#11691).

Revision 10240
Added by Jean-Philippe Lang about 5 years ago

Fixed that destroying a user from the edit page returns a 404 response (#11691).

History

#1 Updated by Etienne Massip about 5 years ago

  • Status changed from Confirmed to Resolved
  • Target version changed from Candidate for next minor release to 2.1.0
  • Resolution set to Fixed

Should be fixed with r10234.

Note that I had to revert r1893 (#1826) because URI.parse is expecting an escaped URL (try to give it some UTF-8 encoded character and it will throw an exception).

#2 Updated by Etienne Massip about 5 years ago

Yes I think they should have because r1893 was committed on a "can't reproduce" basis and broke any possibilities to include a UTF-8 encoded parameter in the URL (was utf-8 RoR param in this case).
I don't see the point of pre-escaping a parameter which will be necessarily URL-escaped at the time the request is issued.
And I think that #1826 could have been fixed by using relative URLs which wouldn't have been processed by Apache mod_rewrite.

Could you please discuss them before reverting other people's commits?
It makes 2 times for reasons that could have been discussed before, it's rude, very frustrating and not really motivating.

#3 Updated by Jean-Philippe Lang about 5 years ago

Etienne Massip wrote:

Yes I think they should have

Sure but changing a test just to make it pass is not the way to go either. Updating and getting an error when logging in is pretty frustrating too :-(

#4 Updated by Jean-Philippe Lang about 5 years ago

  • Subject changed from Redirected to user edit page after its deletion (404 error) to 404 response when deleting a user from the edit page
  • Status changed from Resolved to Closed
  • Assignee set to Jean-Philippe Lang

#5 Updated by Jean-Philippe Lang about 5 years ago

Could you please discuss them before reverting other people's commits?

OK, sorry for that. Next time, let's discuss before actually starting to do the changes that should be discussed. Thanks for digging into this anyway Etienne.

#6 Updated by Etienne Massip about 5 years ago

Jean-Philippe Lang wrote:

Sure but changing a test just to make it pass is not the way to go either. Updating and getting an error when logging in is pretty frustrating too :-(

I did not change the test just to make it pass but because it was part of r1893 which I reverted; the after-login redirect was then expected to work just as it did before r1893 (or a test failure) but as you discovered it was not the case and I'm really sorry I broke it :(

Next time, let's discuss before actually starting to do the changes that should be discussed.

I did not expect any side-effect but as I updated common code I still added you as a watcher and didn't merge anything to branches or closed the issue.

As a regression, shouldn't it be fixed in 1.4 and 2.0 branches as well?

Also available in: Atom PDF