8000 Fixed getBatchEntities by lukassykora · Pull Request #9745 · mautic/mautic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fixed getBatchEntities #9745

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.

Sign up for GitHub

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
Merged
merged 3 commits into from
Apr 30, 2021
Merged

Fixed getBatchEntities #9745

merged 3 commits into from
Apr 30, 2021

Conversation

lukassykora
Copy link
Contributor
@lukassykora lukassykora commented Mar 2, 2021 •
edited
Loading
Q A
Branch? 3.3
Bug fix? yes
New feature? no
Deprecations? no
BC breaks? no
Automated tests included? yes
Related user documentation PR URL
Related developer documentation PR URL changes -->
Issue(s) addressed #9621

Description:

The batch endpoint PATCH api/users/batch/edit shows inconsistent behavior. If often reports the following errors: username: Username is already in use. Please choose another., email: Email is already in use. Please choose another. - however, this is not consistent. Sometimes the action succeeds.

Steps to test this PR:

  1. Load up this PR
  2. The testing scenario is described in Inconsisent behavior in batch edit users action #9621

Clone this minimal repo I created: https://github.com/dennisameling/mautic-users-batch-repo
Follow the steps in the repo to reproduce

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Mar 2, 2021
@lukassykora lukassykora mentioned this pull request Mar 2, 2021
Closed
@codecov
Copy link
codecov bot commented Mar 2, 2021 •
edited
Loading

Codecov Report

Merging #9745 (6f2b12f) into features (f100c3c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             features    #9745   +/-   ##
===========================================
  Coverage       39.91%   39.91%           
- Complexity      34335    34336    +1     
===========================================
  Files            2038     2038           
  Lines          108603   108606    +3     
===========================================
+ Hits            43346    43349    +3     
  Misses          65257    65257
Impacted Files Coverage Δ Complexity Δ
...ndles/ApiBundle/Controller/CommonApiController.php 72.94% <100.00%> (+0.06%) 159.00 <0.00> (ø)
...bundles/ApiBundle/Helper/BatchIdToEntityHelper.php 98.41% <100.00%> (+0.05%) 23.00 <1.00> (+1.00)

@RCheesley RCheesley added API Anything related to the API bug Issues or PR's relating to bugs T1 Low difficulty to fix (issue) or test (PR) labels Mar 2, 2021
@RCheesley RCheesley mentioned this pull request Mar 8, 2021
Closed
@alanhartless alanhartless added this to the 4.0-beta milestone Apr 24, 2021
@alanhartless alanhartless force-pushed the Fix-API-3.3-edit-entities-behaviour-described-in-9621 branch from 9df42de to a6087c2 Compare April 25, 2021 14:15
@alanhartless alanhartless changed the base branch from 3.3 to features April 25, 2021 14:16
@alanhartless alanhartless force-pushed the Fix-API-3.3-edit-entities-behaviour-described-in-9621 branch from a6087c2 to a82139f Compare April 25, 2021 14:17
@alanhartless
Copy link
Contributor

I rebased this on features (M4), fixed the failing test due to new code in features, then re-enabled the skipped tests in the API library to test this.

Screen Shot 2021-04-25 at 9 45 43 AM

The batch API tests are passing again.

I also ran @dennisameling script and the users were updated:

php index.php
string(598) "[{"username":"API_hg3f5uVj","firstName":"APIAPI_hg3f5uVj","lastName":"TestAPI_hg3f5uVj","email":"API_hg3f5uVj@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_Rx2MYkyz","firstName":"APIAPI_Rx2MYkyz","lastName":"TestAPI_Rx2MYkyz","email":"API_Rx2MYkyz@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_x3MzmvQP","firstName":"APIAPI_x3MzmvQP","lastName":"TestAPI_x3MzmvQP","email":"API_x3MzmvQP@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1}]"
Successfully created user with ID 8 and username API_hg3f5uVj
Successfully created user with ID 9 and username API_Rx2MYkyz
Successfully created user with ID 10 and username API_x3MzmvQP
string(620) "[{"username":"API_hg3f5uVj","firstName":"APIAPI_hg3f5uVj","lastName":"TestAPI_hg3f5uVj","email":"API_hg3f5uVj@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"i
8000
d":8},{"username":"API_Rx2MYkyz","firstName":"APIAPI_Rx2MYkyz","lastName":"TestAPI_Rx2MYkyz","email":"API_Rx2MYkyz@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":9},{"username":"API_x3MzmvQP","firstName":"APIAPI_x3MzmvQP","lastName":"TestAPI_x3MzmvQP","email":"API_x3MzmvQP@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":10}]"
Successfully updated 3 users

php index.php
string(598) "[{"username":"API_oNmgxivk","firstName":"APIAPI_oNmgxivk","lastName":"TestAPI_oNmgxivk","email":"API_oNmgxivk@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_fzvcikOB","firstName":"APIAPI_fzvcikOB","lastName":"TestAPI_fzvcikOB","email":"API_fzvcikOB@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_hsOypf0x","firstName":"APIAPI_hsOypf0x","lastName":"TestAPI_hsOypf0x","email":"API_hsOypf0x@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1}]"
Successfully created user with ID 11 and username API_oNmgxivk
Successfully created user with ID 12 and username API_fzvcikOB
Successfully created user with ID 13 and username API_hsOypf0x
string(622) "[{"username":"API_oNmgxivk","firstName":"APIAPI_oNmgxivk","lastName":"TestAPI_oNmgxivk","email":"API_oNmgxivk@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":11},{"username":"API_fzvcikOB","firstName":"APIAPI_fzvcikOB","lastName":"TestAPI_fzvcikOB","email":"API_fzvcikOB@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":12},{"username":"API_hsOypf0x","firstName":"APIAPI_hsOypf0x","lastName":"TestAPI_hsOypf0x","email":"API_hsOypf0x@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":13}]"
Successfully updated 3 users


php index.php
string(598) "[{"username":"API_nziwBPIa","firstName":"APIAPI_nziwBPIa","lastName":"TestAPI_nziwBPIa","email":"API_nziwBPIa@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_EURraijk","firstName":"APIAPI_EURraijk","lastName":"TestAPI_EURraijk","email":"API_EURraijk@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_nSTl8hZD","firstName":"APIAPI_nSTl8hZD","lastName":"TestAPI_nSTl8hZD","email":"API_nSTl8hZD@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1}]"
Successfully created user with ID 14 and username API_nziwBPIa
Successfully created user with ID 15 and username API_EURraijk
Successfully created user with ID 16 and username API_nSTl8hZD
string(622) "[{"username":"API_nziwBPIa","firstName":"APIAPI_nziwBPIa","lastName":"TestAPI_nziwBPIa","email":"API_nziwBPIa@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":14},{"username":"API_EURraijk","firstName":"APIAPI_EURraijk","lastName":"TestAPI_EURraijk","email":"API_EURraijk@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":15},{"username":"API_nSTl8hZD","firstName":"APIAPI_nSTl8hZD","lastName":"TestAPI_nSTl8hZD","email":"API_nSTl8hZD@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":16}]"
Successfully updated 3 users

php index.php
string(598) "[{"username":"API_TVuZ8EwI","firstName":"APIAPI_TVuZ8EwI","lastName":"TestAPI_TVuZ8EwI","email":"API_TVuZ8EwI@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_YtqyHcs4","firstName":"APIAPI_YtqyHcs4","lastName":"TestAPI_YtqyHcs4","email":"API_YtqyHcs4@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1},{"username":"API_7iXfGPz1","firstName":"APIAPI_7iXfGPz1","lastName":"TestAPI_7iXfGPz1","email":"API_7iXfGPz1@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1}]"
Successfully created user with ID 17 and username API_TVuZ8EwI
Successfully created user with ID 18 and username API_YtqyHcs4
Successfully created user with ID 19 and username API_7iXfGPz1
string(622) "[{"username":"API_TVuZ8EwI","firstName":"APIAPI_TVuZ8EwI","lastName":"TestAPI_TVuZ8EwI","email":"API_TVuZ8EwI@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":17},{"username":"API_YtqyHcs4","firstName":"APIAPI_YtqyHcs4","lastName":"TestAPI_YtqyHcs4","email":"API_YtqyHcs4@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":18},{"username":"API_7iXfGPz1","firstName":"APIAPI_7iXfGPz1","lastName":"TestAPI_7iXfGPz1","email":"API_7iXfGPz1@email.com","plainPassword":{"password":"topSecret007","confirm":"topSecret007"},"role":1,"id":19}]"
Successfully updated 3 users

@alanhartless alanhartless added the pending-test-confirmation PR's that require one test before they can be merged label Apr 25, 2021
dennisameling
dennisameling approved these changes Apr 26, 2021
View reviewed changes
Copy link
Member
@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Code LGTM! Thanks!

@dennisameling dennisameling added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Apr 26, 2021
@RCheesley RCheesley merged commit c806115 into mautic:features Apr 30, 2021
@dennisameling dennisameling linked an issue May 3, 2021 that may be closed by this pull request
Inconsisent behavior in batch edit users action #9621
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alanhartless alanhartless alanhartless approved these changes

@dennisameling dennisameling dennisameling approved these changes

Assignees
No one assigned
Labels
API Anything related to the API bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T1 Low difficulty to fix (issue) or test (PR)
Projects
None yet
Milestone
4.0.0-beta
Development

Successfully merging this pull request may close these issues.

Inconsisent behavior in batch edit users action
5 participants
@lukassykora @alanhartless @dennisameling @RCheesley @anton-vlasenko
0