AkkomaGang/akkoma-fe
5
13
Fork 64
Code Issues 81 Pull requests 19 Projects Releases 1 Wiki Activity

Commit list accounts state after difference in removed accounts is determined #82

Merged
floatingghost merged 2 commits from :commit-list-accounts-after-delete into develop 2022-08-01 10:11:40 +00:00
Conversation 2 Commits 2 Files changed 1 +10 -6
Ghost commented 2022-07-30 22:11:36 +00:00
First-time contributor
Copy link

Currently updated account list in editing a timeline list is being committed to the state before account difference is calculated and acted upon in API DELETE call, which results in no difference between comitted and new lists and accounts being impossible to delete from the list as consequence. Also POST call is executed always with full list of accounts, which is not necessary and both calls are performed regardless of presence of changes. This PR moves calucaltion of difference before the commit, and makes POST/DELETE API calls predicated on non-empty difference.

Currently updated account list in editing a timeline list is being committed to the state before account difference is calculated and acted upon in API DELETE call, which results in no difference between comitted and new lists and accounts being impossible to delete from the list as consequence. Also POST call is executed always with full list of accounts, which is not necessary and both calls are performed regardless of presence of changes. This PR moves calucaltion of difference before the commit, and makes POST/DELETE API calls predicated on non-empty difference.
Ghost added 1 commit 2022-07-30 22:11:36 +00:00
Commit list accounts state after difference in removed accounts is determined
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
19ae21d7ab
Ghost commented 2022-07-31 08:17:51 +00:00
Author
First-time contributor
Copy link

Added guards for both POST and DELETE API calls so those will only be executed if there was actually some accounts added or removed. Also had to make sure that store getter would return a copy of internal list, so it won't get modified outside the store, otherwise added would always be empty.

Added guards for both POST and DELETE API calls so those will only be executed if there was actually some accounts added or removed. Also had to make sure that store getter would return a copy of internal list, so it won't get modified outside the store, otherwise `added` would always be empty.
floatingghost reviewed 2022-07-31 21:46:43 +00:00
floatingghost left a comment
Owner
Copy link

one nitpick

seriously though pls do change that bio i don't wanna actually act on any warnings - just... y'know don't make me say it again

one nitpick seriously though pls _do_ change that bio i don't wanna actually act on any warnings - just... y'know don't make me say it again
src/modules/lists.js Outdated
@ -77,3 +81,3 @@
},
findListAccounts: state => id => {
return state.allListsObject[id].accountIds
return state.allListsObject[id].accountIds.slice()
floatingghost commented 2022-07-31 21:46:03 +00:00
Owner
Copy link

the preferred way to duplicate an array is via the ... syntax

return [...state.allListsObject[id].accountIds]

the preferred way to duplicate an array is via the `...` syntax `return [...state.allListsObject[id].accountIds]`
Ghost commented 2022-08-01 10:07:57 +00:00
Author
First-time contributor
Copy link

As far as performance goes, ths is less optimal option. But if you want to prioritize readability, sure.

As far as performance goes, ths is less optimal option. But if you want to prioritize readability, sure.
floatingghost commented 2022-08-01 10:11:11 +00:00
Owner
Copy link

the difference in performance would only actually be relevent if we were dealing with (tens of) thousands of elements, which we're not

the difference in performance would only actually be relevent if we were dealing with (tens of) thousands of elements, which we're not
Ghost marked this conversation as resolved
Ghost force-pushed commit-list-accounts-after-delete from 0d1daa948d to febf0d0c77 2022-08-01 10:07:44 +00:00 Compare
floatingghost merged commit f474763151 into develop 2022-08-01 10:11:40 +00:00
Ghost commented 2022-08-01 10:15:18 +00:00
Author
First-time contributor
Copy link

one nitpick

seriously though pls do change that bio i don't wanna actually act on any warnings - just... y'know don't make me say it again

Sure thing. Done.

> one nitpick > > seriously though pls _do_ change that bio i don't wanna actually act on any warnings - just... y'know don't make me say it again Sure thing. Done.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
a11y

Issues and PRs about accessibility

  
Bug

Applies to issues describing bugs

  
Bug fix

Pull requests that squash bugs

  
Critical Priority

   
Documentation

  
Feature

Pull requests that implement new features

  
Feature request

Issues that request new features

  
Held for next release cycle

PR reviewed during release candidate phase, approved and held as PR

  
High Priority

   
Low Priority

   
Medium Priority

   
Minor change

For minor changes, ie CSS alignment changes

  
Translation/Locale

For changes related to translations

  
WIP

Work in progress

No labels
a11y
Bug
Bug fix
Critical Priority
Documentation
Feature
Feature request
Held for next release cycle
High Priority
Low Priority
Medium Priority
Minor change
Translation/Locale
WIP
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma-fe#82
No description provided.
Delete branch ":commit-list-accounts-after-delete"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?