Ref T11217. This just adds the table that we'll store tokens in. It doesn't make use of the table at all yet. This is mostly pulled from this diff (D16178). Specifically I mostly followed Evan's instructions related to the token table here: D16178#189120.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T11217: Make Tokens modular / real application
- Commits
- rP32d660c08f46: Added a `token_token` table in anticipation of some data-driven tokens
I ran ./bin/storage upgrade successfully and there were no schema errors.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Couple of possible things inline, but this looks pretty good to me.
| resources/sql/autopatches/20160928.tokentoken.sql | ||
|---|---|---|
| 12 | This should maybe be nullable since I think builtin tokens probably won't use an actual File for their data (we can just read it directly off disk) but we can fix that easily later and I'm not 100% sure it will actually be how things turn out. | |
| src/applications/tokens/storage/PhabricatorTokensToken.php | ||
| 25–27 | (Column doesn't exist?) | |
| 98–100 | Let's just omit this for now -- we don't have a mailKey, and I think interacting with tokens via email seems a little silly / not worth building, at least for now. I could be wrong and maybe emailing datboi@whatever.phabricator.com will be all the rage in a few months, but doesn't feel like the highest-value interaction here. | |
| 112–117 | I think this isn't right -- it's loading copies of itself by a nonexistent column. Probably intended to delete PhabricatorTokenGiven instead (replace new self() with new PhabricatorTokenGiven())? I think we could reasonably delete the File by filePHID, if filePHID is set too. | |
| 126 | This is ultimately a product question but maybe creating a token shouldn't permanently auto-subscribe you to it. We don't really have a hard-and-fast rule for this but I think the soft rule is "if you own the object", and a creator's ownership claim on a token feels a little tenuous (e.g., compared to a Differential Revision, which is clearly strongly-owned). We recently backed off on auto-subscribe on Phame blogs, and this ownership feels similarly soft to me. We can easily tweak this later, though. | |
| src/applications/tokens/storage/PhabricatorTokensToken.php | ||
|---|---|---|
| 126 | What does it even mean to be subscribed to a token? I can't think of a scenario where I would want that. | |