Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Albert Volkman’s picture

Status: Active » Needs review
aspilicious’s picture

Status: Needs review » Needs work
+++ b/core/modules/user/config/user.settings.ymlundefined
@@ -12,3 +12,6 @@ notify:
+block_max_list_count: '10'
+block_seconds_online: '900'
+block_whois_new_count: '5'
diff --git a/core/modules/user/user.module b/core/modules/user/user.module

Group this in a block key. Like block.max_list_count and block.seconds_online

asaal’s picture

Assigned: Unassigned » asaal
asaal’s picture

The patch doesn't work on root system of drupal.

I checked out a new branch and apply the following line in command shell:

'git apply -v user_block_cmi.patch'

dlamp@dlamp-vm-server:~/workspace/drupal/drupal-8x-dev/htdocs$ git apply -v user_block_cmi.patch
Checking patch core/modules/user/config/user.settings.yml...
warning: core/modules/user/config/user.settings.yml has type 100755, expected 100644
error: while searching for:
register_pending_approval: '1'
register: visitors
signatures: '0'

error: patch failed: core/modules/user/config/user.settings.yml:12
error: core/modules/user/config/user.settings.yml: patch does not apply
Checking patch core/modules/user/user.module...
Hunk #1 succeeded at 722 (offset -41 lines).
Hunk #2 succeeded at 740 (offset -41 lines).
Hunk #3 succeeded at 805 (offset -41 lines).
Hunk #4 succeeded at 821 (offset -41 lines).
Hunk #5 succeeded at 835 (offset -41 lines).

but it doesn't work and the patch was not applied.

I make change manually from patch above and create new one.

asaal’s picture

Assigned: asaal » Unassigned
Status: Needs work » Needs review
FileSize
5.51 KB

It is fixed and needs review.

asaal’s picture

Assigned: Unassigned » asaal
penyaskito’s picture

Status: Needs review » Needs work

Needs an upgrade path. See http://drupal.org/node/1667896#upgrade

penyaskito’s picture

Issue tags: +Needs upgrade path

Tagging accordingly.

asaal’s picture

Status: Needs work » Needs review
FileSize
5.82 KB

I have add a upgrade path and new yml file for separate the user_block_* settings.

Hanspolo’s picture

This patch works fine for me.
Variables are stored in config and the blocks work as they did before.

Hanspolo’s picture

Status: Needs review » Reviewed & tested by the community

This patch works fine for me.
Variables are stored in config and the blocks work as they did before.

longwave’s picture

+        $count = config('user.block')->get('whois_new_count');
+        $interval = REQUEST_TIME - config('user.block')->get('seconds_online');
+        $max_users = config('user.block')->get('max_list_count');

Minor nitpick, these should probably use a common $config = config('user.block'); as in user_block_save().

Also, do we need tests for the upgrade path?

asaal’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.06 KB

Added the minor nitpick. ;-)

Hanspolo’s picture

Status: Needs review » Reviewed & tested by the community

The new patch works as well as #9.
The points of #12 are fixed.

I think that tests are not needed, because the functionality doesn't change.

webflo’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.34 KB
10.22 KB

Reverted the changed file permissions from the patch in #13. I found one variable_get in the test and converted this to cmi.

webflo’s picture

Ooops. Uploaded the wrong patch.

webflo’s picture

Status: Needs review » Needs work
Issue tags: -Needs upgrade path

The last submitted patch, user-convert_user_block_variables_to_cmi-1823348-17.patch, failed testing.

webflo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, user-convert_user_block_variables_to_cmi-1823348-17.patch, failed testing.

webflo’s picture

Status: Needs work » Needs review
Issue tags: +Needs upgrade path
asaal’s picture

Hi @webflo thanks for reviewing and changing the test case.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

done!

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

sdboyer’s picture

fyi, this patch was kinda pointless, as this is all getting refactored: #1535868: Convert all blocks into plugins

oh well, no big.

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