X Tutup
Skip to content

Respect default app config within the TwoFactorChallendeController#36031

Merged
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/respect-defaultapp-in-TwoFactorChallengeController
Aug 14, 2019
Merged

Respect default app config within the TwoFactorChallendeController#36031
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/respect-defaultapp-in-TwoFactorChallengeController

Conversation

@DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Aug 14, 2019 •
edited
Loading

Description

Within the TwoFactorChallengeController the default app setting is not respected

Related Issue

❓

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@DeepDiver1975 DeepDiver1975 requested a review from xoxys August 14, 2019 10:01
@DeepDiver1975 DeepDiver1975 self-assigned this Aug 14, 2019
@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/respect-defaultapp-in-TwoFactorChallengeController branch from 58424c3 to 220be42 Compare August 14, 2019 11:42
@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

Merging #36031 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #36031   +/-   ##
=======================================
  Coverage   53.85%   53.85%           
=======================================
  Files          63       63           
  Lines        7377     7377           
  Branches     1301     1301           
=======================================
  Hits         3973     3973           
  Misses       3019     3019           
  Partials      385      385
Flag Coverage Δ
#javascript 53.85% <ø> (ø) ⬆️

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 2908c40...220be42. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 14, 2019 •
edited
Loading

Codecov Report

Merging #36031 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #36031   +/-   ##
=======================================
  Coverage   53.85%   53.85%           
=======================================
  Files          63       63           
  Lines        7377     7377           
  Branches     1301     1301           
=======================================
  Hits         3973     3973           
  Misses       3019     3019           
  Partials      385      385
Flag Coverage Δ
#javascript 53.85% <ø> (ø) ⬆️

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 2908c40...5e9284c. Read the comment docs.

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/respect-defaultapp-in-TwoFactorChallengeController branch from 220be42 to 5e9284c Compare August 14, 2019 12:15
@DeepDiver1975 DeepDiver1975 merged commit dbb0ee4 into master Aug 14, 2019
@delete-merged-branch delete-merged-branch bot deleted the bugfix/respect-defaultapp-in-TwoFactorChallengeController branch August 14, 2019 13:38
@davitol davitol mentioned this pull request Sep 3, 2019
Closed
13 tasks
@davitol
Copy link
Contributor

davitol commented Sep 12, 2019 •
edited
Loading

@DeepDiver1975 are these test steps fine?
1- Set in config.php 'defaultapp' => 'market'.
2- Log in with 2FA and check market is shown at login,

(I just want to confirm the purpose of the PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@xoxys xoxys xoxys approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@DeepDiver1975 DeepDiver1975

Labels

3 - To Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DeepDiver1975 @davitol @xoxys @patrickjahns
X Tutup