Ref T4896. Move all direct accesses to the inline comment table behind a small amount of API to make it easier to migrate the table.
Details
Details
- Reviewers
joshuaspence btrahan - Maniphest Tasks
- T4896: Move Audit to ApplicationTransactions
- Commits
- Restricted Diffusion Commit
rP8605a1808d01: Hide direct accesses to Audit inline comment table behind API
- Grepped for PhabricatorAuditInlineComment.
- Grepped for audit_inlinecomment.
- Created a draft comment.
- Previewed a draft comment.
- Reloaded page, still saw draft.
- Viewed standalone, still saw draft.
- Made comment, inline published.
- Added a draft, saw both.
- Edited inline comment.
- Reindexed commit.
- Searched for unique word in published comment, found commit.
- Searched for unique word in draft comment, no results.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Some minor inlines.
| src/applications/audit/storage/PhabricatorAuditInlineComment.php | ||
|---|---|---|
| 21 | Consider $author instead of $viewer? | |
| 31 | Unused variable? I'm guessing you put this here to make the API consistent. I'm OK with that. | |
| 42 | This seems a little odd that this method accepts a $path_id parameter whereas loadDraftComments and loadPublishedComments does not. | |
| src/applications/diffusion/controller/DiffusionCommitController.php | ||
| 328 | I think this should be $commit->getPHID() | |
Comment Actions
- Fix $commit instead of $commit->getPHID().
- Forced Diffusion into the "selected changesets" mode to actually test this codepath.
- API oddness is just for consistency, since it's temporary and having it work the same way reduces the chance of me screwing something up. The table access may swap to a Query that requires a real viewer later on.