changes editUserAccount to updateUserAccount #5274

gabru-md · 2018-06-12T18:45:03Z

changes editUserAccount to updateUserAccount in all files and tests.
changes all functions and tests that match with editUser to updateUser

This PR relates to an existing open issue and there are no existing
PRs open for the same issue.
Add Fixes #ISSUENUM at the top of your PR.
Add a description of the the changes introduced in this PR.

Cheers!

codecov-io · 2018-06-12T19:11:31Z

Codecov Report

Merging #5274 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    mozilla/addons-frontend#5274   +/-   ##
=======================================
  Coverage   97.02%   97.02%           
=======================================
  Files         217      217           
  Lines        5521     5521           
  Branches     1078     1078           
=======================================
  Hits         5357     5357           
  Misses        145      145           
  Partials       19       19

Impacted Files	Coverage Δ
src/amo/api/users.js	`100% <ø> (ø)`	⬆️
src/amo/components/UserProfileEdit/index.js	`100% <ø> (ø)`	⬆️
src/amo/reducers/users.js	`100% <100%> (ø)`	⬆️
src/amo/sagas/users.js	`100% <100%> (ø)`	⬆️

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 ca49275...509157b. Read the comment docs.

gabru-md · 2018-06-13T14:00:04Z

@willdurand is there something more to be done?

bobsilverberg

Thanks for taking this on, @gabru-md. There are a few more places where "editUserAccount" is referenced, for example there's finishEditUserAccount, FinishEditUserAccountParams, FinishEditUserAccountAction, EditUserAccountParams, EditUserAccountAction, and FINISH_EDIT_USER_ACCOUNT.

All of these should also be renamed.

gabru-md · 2018-06-14T16:36:43Z

okay @bobsilverberg . I'll make the changes 👍

gabru-md · 2018-06-14T17:57:02Z

@bobsilverberg I've done the changes.
Travis is passing as well. 😃

bobsilverberg

This looks good, nice work @gabru-md. We're almost there. In addition to my comments below, I noticed that there's still a helper function in TestUserProfileEdit.js called _editUserAccount. That should be renamed to _updateUserAccount.

bobsilverberg · 2018-06-15T17:32:12Z

tests/unit/amo/sagas/test_users.js

-  describe('editUserAccount', () => {
-    it('calls the API to edit a user after editUserAccount()', async () => {
+  describe('updateUserAccount', () => {
+    it('calls the API to edit a user after updateUserAccount()', async () => {


This should probably change to calls the API to update a user....

bobsilverberg · 2018-06-15T17:35:56Z

src/amo/sagas/users.js

@@ -76,7 +76,7 @@ export function* editUserAccount({
    log.warn(`Could not edit user account: ${error}`);


This should be changed to Could not update user account....

bobsilverberg · 2018-06-15T20:04:44Z

tests/unit/amo/api/test_users.js

@@ -50,7 +50,7 @@ describe(__filename, () => {
    });
  });

-  describe('editUserAccount', () => {
+  describe('updateUserAccount', () => {
    const getParams = (params = {}) => {
      const state = dispatchSignInActions().store.getState();
      const userId = getCurrentUser(state.users).id;


There's a line below (line 61), which should be changed from it('edits a userProfile and... to it('updates a userProfile and....

This test description still needs to be updated in tests/unit/amo/api/test_users.js .

bobsilverberg · 2018-06-15T20:13:19Z

tests/unit/amo/sagas/test_users.js

@@ -244,7 +244,7 @@ describe(__filename, () => {
        .once()
        .returns(Promise.resolve(allNotifications));

-      sagaTester.dispatch(editUserAccount({
+      sagaTester.dispatch(updateUserAccount({
        errorHandlerId: errorHandler.id,
        notifications,
        picture: null,


There's a line below (line 266), on which the description should be changed from it('cancels the edit and dispatches... to it('cancels the update and dispatches ....

bobsilverberg

This looks great, thanks @gabru-md. There's just one comment from the last time I reviewed this, about a description in a test, which still needs to be addressed. Once that's fixed this should be good to land.

bobsilverberg · 2018-06-18T21:07:09Z

tests/unit/amo/api/test_users.js

@@ -50,7 +50,7 @@ describe(__filename, () => {
    });
  });

-  describe('editUserAccount', () => {
+  describe('updateUserAccount', () => {
    const getParams = (params = {}) => {
      const state = dispatchSignInActions().store.getState();
      const userId = getCurrentUser(state.users).id;


This test description still needs to be updated in tests/unit/amo/api/test_users.js .

gabru-md · 2018-06-22T16:41:51Z

@bobsilverberg is there anything left?

bobsilverberg

This looks great, thanks for all your work on this @gabru-md, and sorry for my delay in getting back to you.

The only issue now is that this has conflicts with our current master branch, and those need to be resolved by rebasing it on top of master. If you are comfortable doing that, please do so and ping me when it's done. Otherwise I'd be happy to do it for you and land the code.

gabru-md · 2018-06-26T19:28:05Z

@bobsilverberg I'm done with the rebase as well as the Travis Fixes.
I've also squashed all of my commits. Perhaps you can take a look 😄

bobsilverberg

Looks great, thanks so much @gabru-md. Nice work! 👍

gabru-md · 2018-06-27T12:29:03Z

Thanks You @bobsilverberg
I would love to take up more issues and help the community 👍

bobsilverberg suggested changes Jun 14, 2018

View reviewed changes

bobsilverberg self-assigned this Jun 14, 2018

gabru-md force-pushed the gabru-md/patch branch from 3c2223e to dfa353c Compare June 14, 2018 17:07

bobsilverberg suggested changes Jun 15, 2018

View reviewed changes

bobsilverberg suggested changes Jun 18, 2018

View reviewed changes

bobsilverberg approved these changes Jun 26, 2018

View reviewed changes

gabru-md force-pushed the gabru-md/patch branch from bf431cd to 85df1d5 Compare June 26, 2018 19:17

changes all functions and tests that match with editUser to updateUser

509157b

gabru-md force-pushed the gabru-md/patch branch from b531988 to 509157b Compare June 26, 2018 19:26

bobsilverberg approved these changes Jun 26, 2018

View reviewed changes

bobsilverberg merged commit d1b2f7e into mozilla:master Jun 26, 2018

gabru-md deleted the gabru-md/patch branch June 27, 2018 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

changes editUserAccount to updateUserAccount #5274

changes editUserAccount to updateUserAccount #5274

gabru-md commented Jun 12, 2018 •

edited

Loading

codecov-io commented Jun 12, 2018 •

edited

Loading

gabru-md commented Jun 13, 2018

bobsilverberg left a comment

gabru-md commented Jun 14, 2018

gabru-md commented Jun 14, 2018

bobsilverberg left a comment

bobsilverberg Jun 15, 2018

bobsilverberg Jun 15, 2018

bobsilverberg Jun 15, 2018

bobsilverberg Jun 18, 2018

bobsilverberg Jun 15, 2018

bobsilverberg left a comment

bobsilverberg Jun 18, 2018

gabru-md commented Jun 22, 2018

bobsilverberg left a comment

gabru-md commented Jun 26, 2018

bobsilverberg left a comment

gabru-md commented Jun 27, 2018

		@@ -76,7 +76,7 @@ export function* editUserAccount({
		log.warn(`Could not edit user account: ${error}`);

changes editUserAccount to updateUserAccount #5274

changes editUserAccount to updateUserAccount #5274

Conversation

gabru-md commented Jun 12, 2018 • edited Loading

codecov-io commented Jun 12, 2018 • edited Loading

Codecov Report

gabru-md commented Jun 13, 2018

bobsilverberg left a comment

Choose a reason for hiding this comment

gabru-md commented Jun 14, 2018

gabru-md commented Jun 14, 2018

bobsilverberg left a comment

Choose a reason for hiding this comment

bobsilverberg Jun 15, 2018

Choose a reason for hiding this comment

bobsilverberg Jun 15, 2018

Choose a reason for hiding this comment

bobsilverberg Jun 15, 2018

Choose a reason for hiding this comment

bobsilverberg Jun 18, 2018

Choose a reason for hiding this comment

bobsilverberg Jun 15, 2018

Choose a reason for hiding this comment

bobsilverberg left a comment

Choose a reason for hiding this comment

bobsilverberg Jun 18, 2018

Choose a reason for hiding this comment

gabru-md commented Jun 22, 2018

bobsilverberg left a comment

Choose a reason for hiding this comment

gabru-md commented Jun 26, 2018

bobsilverberg left a comment

Choose a reason for hiding this comment

gabru-md commented Jun 27, 2018

gabru-md commented Jun 12, 2018 •

edited

Loading

codecov-io commented Jun 12, 2018 •

edited

Loading