hi all

if we use your module, then put on the settings logout per role (how you see on the attached screenshot), then we have the issue, that all roles are affected and not only the selected roles. can you check this? because on this way we cannot use the module if all roles are effected.

thank you for fast helping

CommentFileSizeAuthor
#15 2917570-15.patch4.72 KBajits
#12 2917570-12.patch2.4 KBajits
#9 autologut-setting.png48.85 KBdeepanker_bhalla
#9 authenticated.png7.18 KBdeepanker_bhalla
#6 autologout_logout_per_role_2917570_6.patch1.76 KBmaaty388
#4 autologout_logout_per_role_2917570_4.patch757 bytesnkoporec
#2 Screenshot.png70.68 KBstockticker
2017-10-20_08-58-45.png358.47 KBcola

Comments

cola created an issue. See original summary.

stockticker’s picture

stockticker commented
Priority: Critical » Normal
Status: Active » Closed (works as designed)
StatusFileSize
new Screenshot.png70.68 KB

If role timeout checkbox is enabled, make sure that you enable all of the roles as well and, for instance, if you want some of the roles never log out - set 0 to timeout property.

I have attached screenshot with proper settings.

However, keep in mind, since your roles (for example, editor - see screenshot) will share authenticated user role as well - it will take less value from available timeout options. I'm not sure if it's a bug or it is working like designed.

cola’s picture

cola commented
Status: Closed (works as designed) » Needs work

i cannot accept this answer, because if you uncheck the checkbox, must this role be ignored... so for me is this a bug not work as designd...

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review
StatusFileSize
new autologout_logout_per_role_2917570_4.patch757 bytes

Here is a patch that fixes this issue.So only roles that are checked will be logged out.

Status: Needs review » Needs work

The last submitted patch, 4: autologout_logout_per_role_2917570_4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

maaty388’s picture

maaty388 commented
StatusFileSize
new autologout_logout_per_role_2917570_6.patch1.76 KB

I updated AutologoutTest.php

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review

@matjaz_zavski thanks for the test fix! Re-applied your patch and it's working.Marking as Needs review so other people can test it too.

janez zibelnik’s picture

janez zibelnik commented

Tested the patch provided by @nkoporec and @matjaz_zavski. The patches fix the issue.

deepanker_bhalla’s picture

deepanker_bhalla commented
Status: Needs review » Needs work
StatusFileSize
new authenticated.png7.18 KB
new autologut-setting.png48.85 KB

Hi,

I have applied the patch and the there is still issue with the per role logout issue. As its not taking the specific time mentioned on the roles as desired.

Kindly check the screenshot as per reference.

authenticated.png : It is my authenticated user.

autologout-setting.png : It is the configuration i have done in the module.

deaom’s picture

deaom commented
Status: Needs work » Reviewed & tested by the community

Hi, I tested the patch and it works, the users gets logout as per role time set in the settings page. The problem is in the block display of the time, which is not correct. Will open a new issue regarding that #3023156: Autologout block not displaying time remaining correctly per role.

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Status: Reviewed & tested by the community » Needs work

Thank you for reporting, patches, and the reviews!

I think we could use this issue to optimize the configuration form. It is confusing tbh. I think the following changes would make sense:

  • Show the "Role" based configuration table only when the "Role Timeout" checkbox is checked. The #states look to be in place. However, the table shows up regardless of the statue of the checkbox.
  • Remove the "Enable" checkbox from all the roles, and default the "Timeout (seconds)" to 0 for all roles on install. With this, if the admin chooses to have time out settings for only some roles they would just need to change the value of those roles. Make the "Timeout (seconds)" field required.
  • Modify any tests which fail due to the change above.

Please let me know if any clarifications are required. Thank you all in advance for your inputs!

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Status: Needs work » Needs review
StatusFileSize
new 2917570-12.patch2.4 KB

I've made a few changes in this patch:

  • The role based table honors the #states for the Role Timeout checkbox.
  • Renamed the misleading Enable checkbox per role to Customize.
  • Removed the authenticated role from the listed role. As the setting is known to fallback on the standard Timeout value in seconds anyways.

Will commit if tests are happy. Don't see any reason why not :)

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Title: Logout per role (setting) not working » Update Logout per role (setting) with clear label

Status: Needs review » Needs work

The last submitted patch, 12: 2917570-12.patch, failed testing. View results

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Status: Needs work » Needs review
StatusFileSize
new 2917570-15.patch4.72 KB

Updated tests.

  • AjitS committed f7f872e on 8.x-1.x 
    Issue #2917570 by AjitS, nkoporec, matjaz_zavski, deepanker_bhalla, cola...
ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Status: Needs review » Fixed

Committed and pushed to 8.x-1.x
Thank you to everyone who was involved!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.