Add feed syntax key

[luacmartins]

Problem

When users select a card feed, we currently select all cards in the feed and use the card: syntax key with those cardIDs. That's worked so far, but it creates an issue where saving this search will save a static list of cardIDs and won't reflect any cards added/removed from the feed.

Why is this important?

Feature should handle a dynamic list of cards

Solution

Keep the UI the same, but use a feed syntax key in search that'll pass the fundID to the backend

cc @SzymczakJ

[melvin-bot]

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[maddylewis]

@luacmartins - since you removed the BUG label, i don't think I'm needed on this issue so I'm going to unassign myself. if im misinterpreting, just lmk!

[JakubKorytko]

Hello my name is Jakub and I work for SWM expert agency. I would like to work on this task, please assign me.

[melvin-bot]

📣 @JakubKorytko! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[JakubKorytko]

@luacmartins, just to be sure, please let me know if my understanding is correct.

1. fundID is (or can be used as) an identifier for feeds, where a feed is basically a group of cards assigned to a specific workspace.
2. If I am correct with the first, then feed syntax key can only be used to filter/search workspace cards, not user (wallet added) nor domain cards.
3. If the user selects all cards from the given feed one by one, but not the feed itself, should the feed syntax key be sent or a list of cards separately?

[luacmartins]

1. fundID is (or can be used as) an identifier for feeds, where a feed is basically a group of cards assigned to a specific workspace.
2. If I am correct with the first, then feed syntax key can only be used to filter/search workspace cards, not user (wallet added) nor domain cards.

AFAIK this is correct. @mountiny to double check me

3. If the user selects all cards from the given feed one by one, but not the feed itself, should the feed syntax key be sent or a list of cards separately?

I believe that right now the behavior is if a user selects all cards on the feed, we select the feed itself. I think we want to keep this behavior.

[Kicu]

Just wanted to confirm an edge case related to:

I believe that right now the behavior is if a user selects all cards on the feed, we select the feed itself. I think we want to keep this behavior.

1. user selects a feed
2. this in turn selects ALL cards belonging to feed
3. user then deselects one of those ☝ cards
4. this in turn deselects the feed

In this scenario we will only save the specific card ids and not the feed - right?

[mountiny]

If I am correct with the first, then feed syntax key can only be used to filter/search workspace cards, not user (wallet added) nor domain cards.

The fundID is equal to domainAccountID - so this works for both workspace and domain feeds.

However, there might be a confusion here. Lets say you:

1. Setup a workspace
2. Enable Expensify Cards and company cards
3. now a domain account is create for the workspace (1:1 relationship) - this is the workspaceAccountID and its equal to the fundID of the domain. let's say the id is 12345
4. Now you setup the Expensify cards - the admin cards will be on: cards_12345_Expensify Card key. The feed unique identifier is the Expensify Card
5. Now they setup Amex cards - the admin cards collection will be on cards_12345_gl10251 key. The unique identifier is gl10251 on the fundID 12345
6. If they add lets say another capital one direct feed, those cards will be on cards_12345_oauth.capitalone.com key where the oauth.capitalone.com is the unique key for the card feed on the 12345 fundID

So in order to identify the correct feed of cards, I think we need to pass both, the fundID (which identifies the domain account - if you set up multiple workspaces with Visa cards, you would have multiple _vcf feeds, but with different fundIDs, since each workspace will have different fundID (workspaceAccountID in the policy object))

This is the same for Domain; however, the user normally only has a single domain based on their email.

If the user selects all cards from the given feed one by one, but not the feed itself, should the feed syntax key be sent or a list of cards separately?

I feel like, for simplicity's sake, if the user goes and selects all the cards one by one, we should not change what the user selected. We should just select all the cardIDs and create the query that way, even if its at the time equal to the entire feed. If new card is added after that though, the search wont be the same - the user did not select the new card as it did not exist yet.

In this scenario we will only save the specific card ids and not the feed - right?

@Kicu I think that is right

[JakubKorytko]

Just to be clear, the original post said that the UI should not be changed, and at the moment it works like this:

1. If the user selects a card one at a time and it matches all the cards in the feed, the feed is selected as well.
2. If the user deselects a card from a feed so that it no longer matches the feed, then the feed is deselected as well.
3. When the feeder is selected, all cards in the feeder are selected.
4. When feed is deselected, all cards in the feed are deselected as well.

Example.mov

And of course this is only visual, because for the query there is always a list of cards by card ID.
Please let me know if I should change anything in this behavior, as I am only focusing on the logic for creating the query and sending it to the backend for now.

As for the domain-related stuff, unfortunately I have no knowledge of that, so I will let others address that.

[mountiny]

Yeah if its already working like that, dont change it.

As for the domain-related stuff, unfortunately I have no knowledge of that, so I will let others address that.

it is important for the syntax. You will need to include both - workspaceAccountID and the feedName to uniquely identify the feed.

[Kicu]

Then let's agree that unique key for a feed, to be saved in the query (or onyx in some cases) will look something like this:
{workspaceAccountID}-{feedName}.
Does that make sense?

[mountiny]

That makes sense to me, but would wait for @luacmartins to confirm

[JakubKorytko]

If we need a feedName and a workspaceAccountID which is basically equal to fundID, my idea is to use keyForList field without cards_ part from an object returned by selecting a feed, and put in the query, because it consists of everything that is needed.

Please, confirm if this approach is correct.

[luacmartins]

That works. So just to confirm, we'd send the following back to the API (following the example in the screenshots above)

{
    op: "eq",
    left: "feed",
    right: "18755223_Expensify Card",
}

Is that right?

[Kicu]

my idea is to use keyForList

Let's just agree on a key structure and create a dedicated key that will be saved and send. keyForList is a prop that is needed because of how React works - let's keep it like that.

and @luacmartins right: "18755223_Expensify Card", - wouldn't there be a hyphen - in between somewhere? I'm not sure how come on the screenshots that @JakubKorytko showed on screen 1 we see {domainName}-{bank} and on the other there is no - at all?

[JakubKorytko]

and @luacmartins right: "18755223_Expensify Card", - wouldn't there be a hyphen - in between somewhere? I'm not sure how come on the screenshots that @JakubKorytko showed on screen 1 we see {domainName}-{bank} and on the other there is no - at all?

Good point, that's my bad, I've attached wrong screenshot of the structure, to clarify:

• keyForList structure for workspace card is taken from Onyx key directly, so it has a structure:
  

  App/src/ONYXKEYS.ts

  Lines 545 to 549 in bf23769

  	/**
  	* Stores the card list for a given fundID and feed in the format: cards_<fundID>_<bankName>
  	* So for example: cards_12345_Expensify Card
  	*/
  	WORKSPACE_CARDS_LIST: 'cards_',

• For domain card, keyForList has a structure:
  

  App/src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersCardPage.tsx

  Lines 144 to 161 in bf23769

  	Object.values(domainFeedsData).forEach((domainFeed) => {
  	const {domainName, bank, correspondingCardIDs} = domainFeed;
  	const isBankRepeating = repeatingBanks.includes(bank);
  	const feedItem = createCardFeedItem({
  	bank,
  	correspondingCardIDs,
  	cardFeedLabel: isBankRepeating ? getDescriptionForPolicyDomainCard(domainName) : undefined,
  	translate,
  	keyForList: `${domainName}-${bank}`,
  	selectedCards,
  	});
  	if (feedItem.isSelected) {
  	selectedFeeds.push(feedItem);
  	} else {
  	unselectedFeeds.push(feedItem);
  	}
  	});

And because of that, we should indeed create a dedicated key, as there is a difference in keyForList for domain and workspace cards, but use one of the structures that are already in use, as @Kicu said.
For me it doesn't really matter which one, if there is a preference between these two structures, let me know, for now I will stick to the one Onyx uses.

That works. So just to confirm, we'd send the following back to the API (following the example in the screenshots above)

Correct.

[mountiny]

+1 to the onyx key structure

[luacmartins]

Yea, let's use the Onyx key structure

[melvin-bot]

@luacmartins, @JakubKorytko Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Kicu]

this is being worked on

[JakubKorytko]

I'm still working on it, I'm very close to completely solving this issue, but I've found a related bug and I need to fix it in order for my solution to work.

[luacmartins]

I gotta get started on the backend portion of this 😅