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

[Feature request]: Remaining bulk actions #1120

Closed
11 tasks done
DonKoko opened this issue Jul 2, 2024 · 10 comments
Closed
11 tasks done

[Feature request]: Remaining bulk actions #1120

DonKoko opened this issue Jul 2, 2024 · 10 comments
Assignees
Labels
Priority: High Issues that are with very high priority User requested feature

Comments

@DonKoko
Copy link
Contributor

DonKoko commented Jul 2, 2024

We should add bulk actions to the other models. Lets start 1 by one so we can progressively release them. We can make PR per model, but keep track of the whole scope here.

Kits

  • Delete
  • Assign custody
  • Release custody

Category

  • Delete - we need to double check that the relations get cleaned up on delete. Should be in the schema

Tags

  • Delete - - we need to double check that the relations get cleaned up on delete. Should be in the schema

Locations

  • Delete - we need to double check that the relations get cleaned up on delete. Should be in the schema

Bookings

  • Delete - - we need to double check that the relations get cleaned up on delete. Should be in the schema
  • Archive
  • Cancel

Custom fields

  • Activate/Deactivate - needs to consider tier when activating and we have to handle the errors

NRM

  • Delete
@jurrejansen
Copy link

jurrejansen commented Jul 5, 2024

@DonKoko

Next one is kind of an edge case I think. I imported assets with kit and custodian resulting in 20 assets being part of a kit and having In custody status. However, all assets are In custody individually and not because the kit was assigned custody and assets inheriting the status. This can also happen without an import.

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 9, 2024

Next one is kind of an edge case I think. I imported assets with kit and custodian resulting in 20 assets being part of a kit and having In custody status. However, all assets are In custody individually and not because the kit was assigned custody and assets inheriting the status. This can also happen without an import.

Yeah i know of this but tbh quite complex to solve. We ignore it for now. Also this is more related to imports.

@rockingrohit9639 can you address the other points please?

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 10, 2024

Okey so everything is resolved and released except this:

When selecting the 20 assets that are in a kit and that have In custody status, Release custody action is blocked. For some reason when I go to next page (pagination) the action is possible

It is quite important we fix this. As far as I understand this happens because we only have the ids and when you are on page 2 we don't have the remaining data. What I would recommend is to update the data structure. We currently store an array of ids.
We can do it in 2 ways:

  1. Array of objects - just store all the objects. Arrays can get a bit slow when they get huge so i am a bit skeptical about this
  2. Object of objects - Have an object where inside are objects with the key being the item.id and the value being the item. This is a pattern I have seen some frameworks use because its way more performant to access the data via the key, than having to use .find or .filter inside the array. This is my preferred option on how to handle it. The type of the collection will be something like this:
type Item = Asset | Kit | TeamMember | ....
type ItemsCollection = Record<Item['id'], Item>

I am curious to hear your opinion. @rockingrohit9639

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 11, 2024

@jurrejansen @carlosvirreira this is all implemented and release.
@jurrejansen can you please test the feedback points you gave and if there is any problem please re-open this issue.

@DonKoko DonKoko closed this as completed Jul 11, 2024
@DonKoko DonKoko unpinned this issue Jul 11, 2024
@jurrejansen
Copy link

@DonKoko I was running through these to do quick tests. And I experienced an issue with bulk cancelling bookings. After loading for quite a while...

  • I tried cancelling 2 Reserved bookings which resulted in an error. However I do receive automated emails saying the bookings were cancelled
    Screenshot 2024-07-15 at 11 06 35
    Screenshot 2024-07-15 at 11 08 41

After checking out 1 of the bookings and trying again to bulk cancel it worked
https://github.com/user-attachments/assets/8b7c7856-66f7-4592-b7d1-c389058ec404

@jurrejansen jurrejansen reopened this Jul 15, 2024
@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 15, 2024

@jurrejansen was this in production or on staging? Do I understand correctly that you received the email for both bookings but only the status of 1 of them got changed?

@jurrejansen
Copy link

@DonKoko this was on app.shelf

Bulk selected 2 bookings with Rerserved status
Performed Cancel bulk action
Got error but received automated mail for both booking saying they were cancelled
Refreshed page
Checked out 1 of 2 bookings
Bulk selected both bookings
Performed cancel bulk actions
Both bookings got cancelled and received automated mails saying they were cancelled again

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 15, 2024

@rockingrohit9639 I also found an issue:
I have filtered by in-custody and I have 2 pages. When I click select all the button is disabled, but that is not correct because actually all assets are in custody

Image

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 15, 2024

@rockingrohit9639 I have implemented a solution for that last point: #1170

@carlosvirreira
Copy link
Contributor

Things to consider / add:

  • Batch updating GPS location (Can be handy if you batch selected assets and moved them to a new location) then you might want to do 1 mass GPS update for the selection
  • Batch updating GPS location could be included on a location level / kit / and batch selection of assets

@DonKoko DonKoko closed this as completed Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Issues that are with very high priority User requested feature
Projects
Status: Done
Development

No branches or pull requests

4 participants