Project

General

Profile

Actions

Defect #28000

closed

Deletion of an LDAP authentication mode may fail silently

Added by Go MAEDA about 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
LDAP
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

Deletion of an LDAP authentication mode fails without any error message if one or more users use it. An error message should be displayed.


Files

Actions #1

Updated by Go MAEDA about 6 years ago

  • Target version set to 4.0.0

Setting target version to 4.0.0.

Actions #2

Updated by Toshi MARUYAMA about 6 years ago

  • Target version deleted (4.0.0)
Actions #3

Updated by Go MAEDA about 6 years ago

Added an assertion in the test.

Actions #4

Updated by Toshi MARUYAMA about 6 years ago

You can use raw English text and I think you can use "assert_select_error".
source:trunk/test/functional/auth_sources_controller_test.rb#L142

Actions #5

Updated by Go MAEDA about 6 years ago

Toshi MARUYAMA wrote:

You can use raw English text and I think you can use "assert_select_error".
source:trunk/test/functional/auth_sources_controller_test.rb#L142

Do you think using raw text is better? I think I18n.t is better because we don't have to update the test when the English translation for error_can_not_delete_auth_source is changed. And we cannot use assert_select_error for the page because there isn't "div#errorExplanation".

Actions #6

Updated by Go MAEDA about 6 years ago

Toshi MARUYAMA wrote:

You can use raw English text and I think you can use "assert_select_error".
source:trunk/test/functional/auth_sources_controller_test.rb#L142

I think that checking the flash hash is a common way to test flash notices. Please see 7.7 Testing flash notices on Rails Guides. Also in Redmine, the same way is used. Examples as follows:

I think that assert_select_error can be used for validation errors brought by ActiveRecord::Errors object but cannot be used for flash hash.

Actions #7

Updated by Toshi MARUYAMA about 6 years ago

Go MAEDA wrote:

Toshi MARUYAMA wrote:

You can use raw English text and I think you can use "assert_select_error".
source:trunk/test/functional/auth_sources_controller_test.rb#L142

Do you think using raw text is better?

Yes. Rails changed behaviour many times. It cannot guarantee test result in the future.

Actions #8

Updated by Jens Krämer about 6 years ago

I agree with Toshi, using the raw text in the assertion is preferable. It simplifies the statement and makes it more obvious what is expected.

Actions #9

Updated by Go MAEDA about 6 years ago

Toshi MARUYAMA and Jens Krämer, thank you for your advice!
I have updated the test to use raw text.

Actions #10

Updated by Go MAEDA about 6 years ago

  • Target version set to 4.0.0

I fixed the patch as advised by Toshi and Jens. Setting target version to 4.0.0 again.

Actions #11

Updated by Go MAEDA about 6 years ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed.

Actions

Also available in: Atom PDF