8000 Bugfix consent dialog by tijsverkoyen · Pull Request #3226 · forkcms/forkcms · GitHub
[go: up one dir, main page]

Skip to content

Bugfix consent dialog#3226

Merged
carakas merged 4 commits intomasterfrom
bugfix-consent-dialog
Nov 13, 2020
Merged

Bugfix consent dialog#3226
carakas merged 4 commits intomasterfrom
bugfix-consent-dialog

Conversation

@tijsverkoyen
Copy link
Member

Type

  • Non critical bugfix

Pull request description

When saving the setting a level 'functional' was stored in the settings. Which is not needed, as the level is there per default.
I also fixed a bug when no levels are defined, this was stored as an empty string.

As the functional level is added through the code it should not be stored in
the settings.
The explode returned an array with an empty string, if there was nothing filled
in.
@tijsverkoyen tijsverkoyen requested a review from a team November 13, 2020 14:22
@carakas carakas added this to the 5.9.2 milestone Nov 13, 2020
);
$privacyConsentLevels = [];
if($privacyConsentLevelsField->isFilled()) {
$privacyConsentLevels = explode(',', $this->form->getField('privacy_consent_levels')->getValue());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use $privacyConsentLevelsField here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b31a09c

@codecov
Copy link
codecov bot commented Nov 13, 2020

Codecov Report

Merging #3226 (b31a09c) into master (e1a7047) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3226      +/-   ##
============================================
- Coverage     28.21%   28.21%   -0.01%     
- Complexity     8013     8014       +1     
============================================
  Files           567      567              
  Lines         30629    30632       +3     
============================================
  Hits           8642     8642              
- Misses        21987    21990       +3     
Flag Coverage Δ Complexity Δ
functional 23.91% <0.00%> (-0.01%) 0.00 <0.00> (ø)
installer 3.88% <0.00%> (-0.01%) 0.00 <0.00> (ø)
unit 7.78% <0.00%> (-0.01%) 0.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
src/Backend/Modules/Settings/Actions/Index.php 0.00% <0.00%> (ø) 53.00 <0.00> (+1.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1a7047...5569783. Read the comment docs.

@tijsverkoyen tijsverkoyen requested a review from carakas November 13, 2020 14:28
$this->form->getField('show_consent_dialog')->getChecked()
);
$privacyConsentLevels = [];
if($privacyConsentLevelsField->isFilled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codestyles also dictate that there needs to be a space after if

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 5569783

@tijsverkoyen tijsverkoyen requested a review from carakas November 13, 2020 14:48
@carakas carakas merged commit e15f850 into master Nov 13, 2020
@carakas carakas deleted the bugfix-consent-dialog branch November 13, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0