Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

do not use login detection bruteforcedetection code during oneClickResults action #17546

Merged
merged 1 commit into from May 11, 2021
Merged

do not use login detection bruteforcedetection code during oneClickResults action #17546

merged 1 commit into from May 11, 2021

Conversation

diosmosis
Copy link
Member

Description:

The one click update feature updates the code, then on the next page view the db upgrade screen is shown. The actions in between these two components use new code, but an old database schema. This PR makes sure we ignore unknown column errors in the bruteforce detection feature if we're in the one click update process.

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@diosmosis diosmosis added the Needs Review PRs that need a code review label May 11, 2021
@diosmosis diosmosis added this to the 4.3.0 milestone May 11, 2021
flamisz
flamisz approved these changes May 11, 2021
View reviewed changes
Copy link
Contributor

@flamisz flamisz left a comment

Choose a reason for hiding this comment

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

I couldn't test it locally (didn't know how to get into this state) but the code looks ok to me, I see how it works and how is it useful. 👍

@diosmosis
Copy link
Member Author

@flamisz just fyi, so you're aware if you have to touch this code: this is for the oneclickupdate feature which is extremely hard to test locally. The feature downloads a package from builds.matomo.org, unzips it and replaces the current version of matomo with it. To test it locally w/o a build on builds.matomo.org you'd have to use the matomo-package repo to build a package from 4.x-dev code, then create a new release channel to get it from localhost, then initiate the process. Personally, I find using the test on travis to be a lot less trouble.

@diosmosis diosmosis merged commit c40c4b3 into 4.x-dev May 11, 2021
@diosmosis diosmosis deleted the fix-build branch May 11, 2021 23:10
@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed Needs Review PRs that need a code review labels May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flamisz flamisz flamisz approved these changes

@tsteur tsteur Awaiting requested review from tsteur

@sgiehl sgiehl Awaiting requested review from sgiehl

Assignees
No one assigned
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@diosmosis @flamisz @sgiehl