[UserPage] Fix tags on BrewItem

[G-Ambatte]

As the tags data object was originally a string (''), when this was converted to an array (I assume using [].concat(tags), it resulted in the first item in the array being an empty string. As the current logic is only to check for the existence of an array (using tags.length > 0), every tags array will result in the logic evaluating to true. When the tags array is populated with data, the first item (tags[0]) will contain actual data.

Instead, we must not only check for the presence of an array, but also that the first item is NOT an empty string.

([].concat(brew.tags)[0]!='') was the logic used in the (now closed) #2330; it should only be a case of readding this logic to the ListPage.jsx logic.

[calculuschild]

Is this a fix that actually needs to go into the Tags part of the Metadata panel so its not erroneously creating an array with an empty first entry in the first place?

If we can cut off the bad data at the source we can keep the logic down the pipeline cleaner.

[G-Ambatte]

I think if we alter the brew save code to eliminate any empty strings from the tags array, then leave in the code to display tags, users will be able to see which brews need to be updated. That's probably easier than trying to update everything at once.

I'll throw some code together in the next day or two.

[ericscheid]

or ... in the code where we retrieve the brew from storage, we do a quick check and eliminate any blank tags (in the state props, not in the mongo/gdrive store).

Thence, code that displays the brew anywhere (e.g. userpage, metadata edit) won't dsplay any blank tags, the user need not even notice these blank tags, the user need do nothing, and when the brew gets saved after any other change the blank tags have already been zapped and don't get saved again.

If a brew is retrieved, blank tags removed, and then not saved .. it doesn't matter because there's always next time.

Even if a broken brew is loaded by some other means (e.g. an API) which evades this fix-when-retrieve strategy and then subsequently gets saved with a blank tag .. it still doesn't matter much because the blank tag will get zapped next time it is retrieved.

[calculuschild]

when this was converted to an array (I assume using [].concat(tags), it resulted in the first item in the array being an empty string

@jeddai Do we know on what line of code this is occurring? Brews that haven't been edited since Tags were released are still just empty quotes, but everything edited since then is being oddly converted to an array with an empty string instead of just an empty array.

[G-Ambatte]

I've been having a quick poke around the code this afternoon...

When the brew is retrieved, any tags value that is a string is converted to an empty array here : https://github.com/naturalcrit/homebrewery/blob/746e8f8a6accf9348c11636d0882442804c1296e/server/homebrew.api.js#L60

I am now wondering if the tags array is initialized to [''] by the Homebrew schema when new HomebrewModel is called, as tags is initialized to [String] : https://github.com/naturalcrit/homebrewery/blob/746e8f8a6accf9348c11636d0882442804c1296e/server/homebrew.model.js#L17

[G-Ambatte]

It won't be affecting my testing on my local version, but maybe a clue: in GoogleActions, tags is a string and not handled with .split like systems is : https://github.com/naturalcrit/homebrewery/blob/746e8f8a6accf9348c11636d0882442804c1296e/server/googleActions.js#L255

[G-Ambatte]

I think I may have found the issue, it looks like tags was still being defined as a string in newPage.jsx. I'm just bringing my fork up to the latest version of master so I can create a branch with a fix.

[G-Ambatte]

Alright, looks like the issue, such as it was, has already been resolved by the latest changes to NewPage.jsx getInitialState and componentDidMount. So future brews won't be affected by the tags[0]='' issue.

[ericscheid]

So .. newPage.jsx won't be creating dud data anymore .. but we might still have dud data saved into mongo or gdrive?

[G-Ambatte]

That's what I've just been looking into... I've put in a tags array filter that eliminates empty strings immediately prior to updating an existing brew; this should also have the benefit of converting any tags string that has made it this far through the piece to an array,

[jeddai]

That seems like a solid solution. Also sorry this entire conversation took place while I was asleep 😬

[calculuschild]

we might still have dud data saved into mongo or gdrive?

Actually.... since Stubs was implemented before the Tags rework, I believe all tags are stored only on Mongo. Yeah... we should theoretically be able to just clean up all the tags directly in the database (using some gradual batch process to avoid freezing the database) and then we don't need a permanent bit of code to catch and clean the duds every time they are loaded. I think there's some MongoDB script that could do that.

Although unless we can figure that out we probably do need that bit of cleaning/filtering code at least temporarily.

[G-Ambatte]

• How many brews ARE there that require updating?
  • Assuming that number is very close to 100%, this essentially resolves to "how many brews are there currently stored in MongoDB?"
• How many brews can we update at once without causing observable server issues?
  • Without knowing anything about the server hardware, and never having used MongoDB in production, it's difficult to hazard a guess. Using Fermi approximation, I imagine that the number is probably bigger than 1k, less than 100k, so I'd estimate 10k. This is based on my previous experience with SQL databases running on admittedly poor hardware; this may not map to MongoDB at all.
• Given the unknown server load at any time, what percentage of the maximum do we want to update in each run?
  • Probably somewhere between 10 - 50 %, so I would look to apply a LIMIT to the update function of between 1,000 to 5,000 records.

https://www.mongodb.com/docs/v5.3/reference/operator/aggregation/limit/

[jeddai]

Actually.... since Stubs was implemented before the Tags rework, I believe all tags are stored only on Mongo.

That's correct! Google brews that have not been stubbed out, however, will still have tags.

[calculuschild]

Google brews that have not been stubbed out, however, will still have tags.

But there was also no way to add tags before now, so every Google brew that has yet to be stubbed should just have an empty string.

How many brews ARE there that require updating?

Probably > 1k but not nearly close to 100%. Should only be brews that were edited between v3.2.0 and v3.2.1 which was 3 days.