In D25967#26245, @avivey wrote:

@aklapper want to land this?

Possible ways to reduce risk for future issues:

• add a Setup Check that runs npm audit
• remove node, use php-based websocket implementation

@aklapper want to land this?
I figure users need to just run npm audit fix to be safe, and then fix the mess it did on the git diff.

Ah...I may take another look (jmeador: Feel of course also very free to commandeer this back to you)

Ah sorry for maybe stepping on toes, and welcome back! :)

Seems @jmeador is AWOL thus updating per last comment

seems @jmeador is AWOL thus boldly commandeering

Digging a bit further, rP600a3e3b7c2de5d93644e0410cd354ea6752949d implies that methods and files should not be indexed at all and not show up in the results. See also T16045#21931.

A quick fix would be removing the single line $query->withIsDocumentable(true); in https://we.phorge.it/source/phorge/browse/master/src/applications/diviner/controller/DivinerFindController.php;85f51c54303fe50ebc09ee0b652033a8a9f29ab1$45
as that'll allow results for Methods.

also manually bump version in package.json; then running npm install results in another bump in package-lock.json

Both are good questions. I only put here the diff which npm created. First question: Very likely Yes.
(Note that I have no knowledge in this area and don't even know why both package-lock.json and package.json are needed.)

1. Can we specify the .10 in packages.json itself?
2. Do installs need upgrade instructions to complete the upgrade?

As a general rule, I prefer the have the abstractions as much as possible, to allow extensions to do things.
In this case, an abstraction would also make this feature easier to enable/disable, which I think is desired.

Chris has asked me to pick this up as he'd like to see this implemented.

If the CSS rule should not change anything, why does it change something?

Yuuum. As I mentioned downstream ( https://phabricator.wikimedia.org/T380361#10778444 ) I think probably, fortunately, nobody noticed this in a real world, since it's complicated to cause this condition using the API, and probably impossible from the web interface, but this still seems somehow a reasonable workflow and not dead code and the change makes sense. Thaaaanks

Remove "return;" lines

Kindly flagging as "little fixes needed on unit tests"

gotta set it

like this I guess

git rebase master

git rebase master

I've added 2 unuseful tips but this seems not ready for production. Flag as "solution seems affected by N+1 query problem". Maybe useful to open a cute task and do more triage.

I've tested in my console:

Relevant report:

Super nice prototype but flagging as "more digging probably needed for production"

oh true that! sigh I should look around a bit more

FYI I pasted the list of the 341 different FA icons used by Phorge as of 2025-04-24 (excluding the list of all icons in PHUIIconView::getIcons()) in P50

Argh, PEBKAC, right. Thanks!

[acko@fedora phorge (master *$|u=)]$ ./bin/storage upgrade
Target                          Error
phabricator_paste.paste.mailKey Surplus