Skip to content

feat: add option to disable favorite iModels fetching - #189

Merged
leo-belanger merged 4 commits into
mainfrom
leobelanger/disable-favorites
Nov 21, 2025
Merged

feat: add option to disable favorite iModels fetching#189
leo-belanger merged 4 commits into
mainfrom
leobelanger/disable-favorites

Conversation

@leo-belanger

@leo-belanger leo-belanger commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

Given that we can hide the favorite icon and the favorite column, we introduce a new option so that we can disable fetching favorite iModels when the user does not care about the feature.

This prevents doing unnecessary calls to the favorites endpoint. This can also prevent unwanted errors when the user does not have access to the favorites endpoint (i.e.: custom shared contract)

Demo

Screen.Recording.2025-11-19.at.08.25.26.mov

@leo-belanger leo-belanger changed the title add option to disable favorites fetching feat: add option to disable favorite iModels fetching Nov 18, 2025
@leo-belanger
leo-belanger marked this pull request as ready for review November 19, 2025 13:52
@aruniverse
aruniverse requested a review from Copilot November 19, 2025 14:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to disable the fetching of favorite iModels, allowing users to opt out of the favorites feature when it's not needed or when they lack access to the favorites endpoint.

Key Changes:

  • Added a disabled prop to prevent fetching favorites when the feature is not used
  • Propagated the disableFavorites option from IModelGrid through IModelFavoritesProvider to useIModelFavorites

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
IModelFavoritesContext.tsx Added disabled prop to the provider and passed it to the hook
useIModelFavorites.ts Added disabled parameter to skip favorites API calls and updated dependency array
IModelGrid.tsx Added disableFavorites prop and passed it to the favorites provider
leobelanger-disable-favorites_2025-11-18-21-33.json Added changelog entry for the new feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DanishMehmood-bit

Copy link
Copy Markdown
Contributor

Question: This feature only works when we select requestType to be "All" OR "". My question, aren't favorite iModels automatically fetched when we set requestType to ""? I know we aren't explicitly calling the favorite iModels api but shouldn't that be the default behavior without adding this disableFavorites option?

@aruniverse Am I missing something?

@DanishMehmood-bit

Copy link
Copy Markdown
Contributor

Also this disableFavorites option is misleading since we are not disabling/removing favorites from the iModels list we are simply not calling favorites api but the default api includes favorites iModels by default and hence my previous comment.
image

@leo-belanger

Copy link
Copy Markdown
Contributor Author

Also this disableFavorites option is misleading since we are not disabling/removing favorites from the iModels list we are simply not calling favorites api but the default api includes favorites iModels by default and hence my previous comment. image

@DanishMehmood-bit Good point, wdyt about merging the options to hide the favourite icon with this new one? Hiding the favourite icon disables the actions so we should not fetch the favourites either.

@leo-belanger

Copy link
Copy Markdown
Contributor Author

Question: This feature only works when we select requestType to be "All" OR "". My question, aren't favorite iModels automatically fetched when we set requestType to ""? I know we aren't explicitly calling the favorite iModels api but shouldn't that be the default behavior without adding this disableFavorites option?

@aruniverse Am I missing something?

@DanishMehmood-bit Favorite iModels are always fetched no matter the request type right now. I'm guessing this is done so we know which iModels are favourite so we can do the correct action (add vs remove). But as you've said in your other comment, we could just merge this option with the hide favourite icon option as a way to disable favourites.

Favourites could still be fetched via the request type "favourite" though. But this is fine since the user has to be explicit about that.

@aruniverse

Copy link
Copy Markdown
Member

Who is this feature for?
Why do they need it?
Do we really want to expose this / allow this?

@leo-belanger

Copy link
Copy Markdown
Contributor Author

Who is this feature for? Why do they need it? Do we really want to expose this / allow this?

@aruniverse This is for work I'm doing. Basically we're using a custom share contract that do not have access to the favorites endpoint (because there is no need to) and we're already disabling the favorite actions with hideFavoriteIcon. But since the favorites are auto fetch right now, we get a bunch of 401s since our contract does not have access to the endpoint. It would be much cleaner to allow disabling the auto fetching of favorites so we don't get these unnecessary failing calls. Like I said in another comment, I can merge my new option with the already existing hideFavoriteIcon option.

@aruniverse

Copy link
Copy Markdown
Member

I can merge my new option with the already existing hideFavoriteIcon option.

I would prefer this then

@aruniverse
aruniverse requested a review from Copilot November 20, 2025 19:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@leo-belanger

Copy link
Copy Markdown
Contributor Author

I would prefer this then

@aruniverse Done!

@leo-belanger
leo-belanger merged commit 9e1d516 into main Nov 21, 2025
5 checks passed
@leo-belanger
leo-belanger deleted the leobelanger/disable-favorites branch November 21, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants