Skip to content

fix User Counts updates on user add/remove#31484

Closed
petre2dor wants to merge 1 commit into
nextcloud:masterfrom
petre2dor:fix/user-counts-update
Closed

fix User Counts updates on user add/remove#31484
petre2dor wants to merge 1 commit into
nextcloud:masterfrom
petre2dor:fix/user-counts-update

Conversation

@petre2dor

Copy link
Copy Markdown

Fix for #27607
I refactored the existing code that update the counts on user enable/disable and added code to update the counts on user add/remove

Signed-off-by: Petre T petre.tudor@dorkfarm.com

Signed-off-by: Petre T <petre.tudor@dorkfarm.com>
@szaimen szaimen added this to the Nextcloud 24 milestone Mar 7, 2022
@szaimen szaimen requested review from a team, Pytal, artonge, blizzz and skjnldsv and removed request for a team March 7, 2022 23:22
@szaimen

szaimen commented Mar 7, 2022

Copy link
Copy Markdown
Contributor

/compile amend /

@skjnldsv

skjnldsv commented Mar 8, 2022

Copy link
Copy Markdown
Member

@petre2dor could you enable us to change this pr? :)

image

@petre2dor

Copy link
Copy Markdown
Author

Hi @skjnldsv,
That checkbox is already checked.
image

@Pytal

Pytal commented Mar 9, 2022

Copy link
Copy Markdown
Member

/compile amend /

@Pytal

Pytal commented Mar 9, 2022

Copy link
Copy Markdown
Member

👆 compile attempt 2

@szaimen

szaimen commented Mar 9, 2022

Copy link
Copy Markdown
Contributor

I guess it would probably make sense to push this to a branch in this repo and invite Petre to the org for easier collaboration...

@szaimen

szaimen commented Mar 9, 2022

Copy link
Copy Markdown
Contributor

I guess it would probably make sense to push this to a branch in this repo and invite Petre to the org for easier collaboration...

@skjnldsv would you be able to do so? :)

@petre2dor

Copy link
Copy Markdown
Author

Hi everyone,
Just a friendly reminder about this PR in the hope we can move this along.

Thanks :)

@szaimen

szaimen commented Mar 22, 2022

Copy link
Copy Markdown
Contributor

@petre2dor thanks for the ping! I would be able to push your changes to a branch in this repo but I fear I am not able to invite you to the org which would mean that you would not be able to push to the new branch...

Apart from that, you can also fix the Node workflow by compiling the js locally and pushing it to the branch.

user.groups.forEach(group => {
// Increment disabled count
state.groups.find(groupSearch => groupSearch.id === group).disabled += enabled ? -1 : 1
// this.commit('updateUserCounts', { user, actionType: enabled ? 'enable' : 'disable' })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would prefer the enable/disable action type so the update one is not wrongly used in the future.

updateUserCounts(state, { user, actionType }) {
const disabledGroup = state.groups.find(group => group.id === 'disabled')
switch (actionType) {
case 'update':

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To not duplicate the code you can handle both enable and disable like so:

		case 'enable':
		case 'disable':
			...

and keeping the current logic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thanks for the review @artonge. I made the chenges and pushed here #31697
please review it again

@skjnldsv

Copy link
Copy Markdown
Member

@skjnldsv would you be able to do so? :)

invited @petre2dor

@szaimen

szaimen commented Mar 24, 2022

Copy link
Copy Markdown
Contributor

invited @petre2dor

@petre2dor did you accept? then I could push your changes to a new branch...

@petre2dor

Copy link
Copy Markdown
Author

invited @petre2dor

@petre2dor did you accept? then I could push your changes to a new branch...

Yes, I did today. Thanks!
I'll make my changes today or tomorrow.

@szaimen

szaimen commented Mar 24, 2022

Copy link
Copy Markdown
Contributor

Yes, I did today. Thanks!
I'll make my changes today or tomorrow.

Shall I push your changes to the new branch in the meantime or do you want to make your new changes first and then push it to the new branch afterwards?

@petre2dor

Copy link
Copy Markdown
Author

Yes, I did today. Thanks!
I'll make my changes today or tomorrow.

Shall I push your changes to the new branch in the meantime or do you want to make your new changes first and then push it to the new branch afterwards?

Please push my changes to a new branch first.
I destroyed my dev machine and I need to rebuild it. No need to use my fork and switch later to Nexcloud repo.

@szaimen

szaimen commented Mar 24, 2022

Copy link
Copy Markdown
Contributor

Superseded by #31697

@szaimen szaimen closed this Mar 24, 2022
@szaimen szaimen removed this from the Nextcloud 24 milestone Mar 24, 2022
@szaimen

szaimen commented Mar 24, 2022

Copy link
Copy Markdown
Contributor

Please push my changes to a new branch first.
I destroyed my dev machine and I need to rebuild it. No need to use my fork and switch later to Nexcloud repo.

done with #31697

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
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.

5 participants