Implements new search engine, projects into Phriction.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T9622: Allow Phriction pages to have Project tags
- Commits
- rP8b4398f9167f: Update PhrictionSearchEngine, implement Projects
Test new search choices, add project to Phriction page. Verify hierarchy still works, setting policy works, etc.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
(I haven't forgotten about this, but I don't remember the order stuff off the top of my head and it's a bit tricky, so I'm going to have to dig into that a little to give you an answer.)
Comment Actions
I think this has a policy problem? See inline.
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 97–124 | Although this code should move to willFilterPage(), I think we still need it. Without this code, I would expect policies to fail to apply correctly to child pages. For example:
| |
| src/applications/phriction/storage/PhrictionDocument.php | ||
| 99–106 | I think we should probably trend away from having attachProjectPHID() methods, since EditEngine and other infrastructure do it for you in modern code and this would have no callers if everything was fully modernized. In legacy code, fine to just EdgeQuery directly when you need this data so everything gets deleted when the code is modernized. Definitely not a big deal, though -- code is fine, just risks not getting cleaned up when we modernize fully. Actually, it looks like none of this has callers even in this diff, since the EditController changes do just use EdgeQuery directly? Just nuke these changes? | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 97–124 | Yeah, I don't know what the fix is here after fiddling with it for a while. It looked like we filter pages already with willFilterPage, How do I integrate this check in with it? | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 171–173 | can't say I understand this either. | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 98–123 | Just copy this whole block into willFilterPage() at the top. willFilterPage() does the check, but it depends on page ancestry data which is provided by this block. If we don't run this code, we never set the correct ancestors on the documents, so when we go to run the check it just looks like every document has no ancestors and doesn't need any special rules/policies. | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 98–123 | How do I get a db connection running? | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 171–173 | The code above uses unset(...) to remove documents from the list, so it's possible we'll reach this point with zero documents left. If the list is empty, this just returns the empty list early before doing more work. If we didn't and needContent was set, we'd issue a bad id IN (<empty>) query on line 176. | |
| src/applications/phriction/query/PhrictionDocumentQuery.php | ||
|---|---|---|
| 98–123 | It's fine to just do this: $table = new PhrictionDocument();
$conn = $table->establishConnection('r');That will properly pool/reuse the connection internally. | |
Comment Actions
Oh I guess we need some inline HTML on {key ...} for email. I tested text mode but not HTML email mode.