Summary: Ref T5952, T3404. This lays the basic plumbing for how this will work, all the way to deploying on Maniphest. Aside from what is mentioned on T5952, I think page(s) on editing application emails could use a little more helpful text about what's going on, similar to how the config page that's getting deprecated works.
Test Plan: ran migration and noted my create email address migrated successfully. used bin/mail to make a task. added another email and used bin/mail to make a task. deleted an email. edited an email. invoked various error states and they all looked good.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T3404, T5952
Differential Revision: https://secure.phabricator.com/D11418
Summary: Fixes T6963. Long term will likely make this more like other document views, but not worth the time right now since this is only location.
Test Plan: Review Phriction document at desktop and mobile breakpoints. Click menu and see menu.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6963
Differential Revision: https://secure.phabricator.com/D11420
Summary:
Taking a pass at revamping the edit pages in Projects. Specifically:
- Remove EditMainController
- Move actions from EditMain to Profile
- Move properties from EditMain to Profile
- Move timeline from EditMain to Profile
- Move Open Tasks from Profile to sidenavicon
- Add custom icons and colors to timeline
Feel free to bang on this a bit and give feedback, feels generally correct to me.
Test Plan: Edit everything I could on various projects. Check links, timelines, actions.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11421
Summary:
Ref T6881. Hopefully, this is the hard part.
This adds a new daemon (the "trigger" daemon) which processes triggers, schedules them, and then executes them at the scheduled time. The design is a little complicated, but has these goals:
- High resistance to race conditions: only the application writes to the trigger table; only the daemon writes to the event table. We won't lose events if someone saves a meeting at the same time as we're sending a reminder out for it.
- Execution guarantees: scheduled events are guaranteed to execute exactly once.
- Support for arbitrarily large queues: the daemon will make progress even if there are millions of triggers in queue. The cost to update the queue is proportional to the number of changes in it; the cost to process the queue is proportional to the number of events to execute.
- Relatively good observability: you can monitor the state of the trigger queue reasonably well from the web UI.
- Modular Infrastructure: this is a very low-level construct that Calendar, Phortune, etc., should be able to build on top of.
It doesn't have this stuff yet:
- Not very robust to bad actions: a misbehaving trigger can stop the queue fairly easily. This is OK for now since we aren't planning to make it part of any other applications for a while. We do still get execute-exaclty-once, but it might not happen for a long time (until someone goes and fixes the queue), when we could theoretically continue executing other events.
- Doesn't start automatically: normal users don't need to run this thing yet so I'm not starting it by default.
- Not super well tested: I've vetted the basics but haven't run real workloads through this yet.
- No sophisticated tooling: I added some basic stuff but it's missing some pieces we'll have to build sooner or later, e.g. `bin/trigger cancel` or whatever.
- Intentionally not realtime: This design puts execution guarantees far above realtime concerns, and will not give you precise event execution at 1-second resolution. I think this is the correct goal to pursue architecturally, and certainly correct for subscriptions and meeting reminders. Events which execute after they have become irrelevant can simply decline to do anything (like a meeting reminder which executes after the meeting is over).
In general, the expectation for applications is:
- When creating an object (like a calendar event) that needs to trigger a scheduled action, write a trigger (and save the PHID if you plan to update it later).
- The daemon will process the event and schedule the action efficiently, in a race-free way.
- If you want to move the action, update the trigger and the daemon will take care of it.
- Your action will eventually dump a task into the task queue, and the task daemons will actually perform it.
Test Plan:
Using a test script like this:
```
<?php
require_once 'scripts/__init_script__.php';
$trigger = id(new PhabricatorWorkerTrigger())
->setAction(
new PhabricatorLogTriggerAction(
array(
'message' => 'test',
)))
->setClock(
new PhabricatorMetronomicTriggerClock(
array(
'period' => 33,
)))
->save();
var_dump($trigger);
```
...I queued triggers and ran the daemon:
- Verified triggers fire;
- verified triggers reschedule;
- verified trigger events show up in the web UI;
- tried different periods;
- added some triggers while the daemon was running;
- examined `phd debug` output for anything suspicious.
It seems to work in trivial use case, at least.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6881
Differential Revision: https://secure.phabricator.com/D11419
Summary: Ref T6822.
Test Plan: `grep`. This method is only called from within `PhutilArgumentWorkflow::__construct`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11415
Summary: Ref T6822. This method needs to be `public` because it is called from `PhabricatorApplicationSearchController::buildApplicationMenu()`.
Test Plan: I wouldn't expect //increasing// method visibility to break anything.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11416
Summary: Ref T6822. This method is only called from `PhutilDaemon::execute()` and can be made `protected`.
Test Plan: See D11404.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11405
Summary: Ref T6822. This method is only called from within the `PhabricatorWorker::executeTask()` and `PhabricatorWorker::scheduleTask()` methods.
Test Plan: `grep`ped for `->doWork`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11406
Summary: Ref T6822.
Test Plan: `grep`. This method is only called from `LiskDAO::update()`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11409
Summary: Ref T6944. This was not quite implemented correctly in D11387.
Test Plan: Saw no more exceptions about being unable to create `/var/log`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: maxhodak, Korvin, epriestley
Maniphest Tasks: T6944
Differential Revision: https://secure.phabricator.com/D11397
Summary: ...also adds policies on who can view and who can edit an action. Fixes T6949.
Test Plan: viewed a secret through the new UI and it worked
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6949
Differential Revision: https://secure.phabricator.com/D11401
Summary: Ref T6962. Mainly accomplished by re-factoring the base editor `buildMailBody` function and then using it differently in the `DifferentialTransactionEditor`.
Test Plan: commented on a revision leaving inline feedback. inspected via bin/mail and it looked good! also made a maniphest comment and checked that email, which still looked good.
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6962
Differential Revision: https://secure.phabricator.com/D11402
Summary: Fixes T6959.
Test Plan: When I was ready to test the feature, the "Daemon & Web config" error already showed up, from having added phd.variant-config. I went meta and changed the value of phd.variant-config to have phd.variant-config. The config error disappeared. I then changed the conpherence setting about conpherence email prefix and the error showed up again. Removing the conpherence config setting made the error disappear once more.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6959
Differential Revision: https://secure.phabricator.com/D11399
Summary: Fixes T6944. Attempt to automatically create the log directory for the Aphlict server. If the directory can't be created, throw a helpful exception.
Test Plan:
# Set `notification.log` to `/var/log/aphlict/aphlict.log`.
# Ran `./bin/aphlict debug` and saw an exception (because the user doesn't have permissions to create the `/var/log/aphlict` directory).
# Ran `sudo chmod 777 /var/log`.
# Ran `./bin/aphlict debug` and saw the `/var/log/aphlict` directory created.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6944
Differential Revision: https://secure.phabricator.com/D11387
Summary: I assume we've shown this long enough, plus with redesign it's a good time to remove.
Test Plan: reload page, no link
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11394
Summary: We still seem to reach for this, though may be time to remove Wiki?
Test Plan: view link
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11393
Summary: In Maniphest, we provide an additional caption shortcut if you can create projects, which has no use if you cant. Fixes T6969
Test Plan: Check page with and without a user's capability.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6969
Differential Revision: https://secure.phabricator.com/D11390
Summary: Ref T6971. This fixes the error the user reported. Not sure what's up with the root cause of their issue.
Test Plan: Went to `/auth/config/new/asdfqwer/` and got a 404 instead of an exception.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6971
Differential Revision: https://secure.phabricator.com/D11388
Summary: Fixes T6955.
Test Plan: made an oauth app. made a test authorization. ran bin/remove destroy <phid of oauth client> and there were no errors. verified oauth app and test authorization were both gone.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6955
Differential Revision: https://secure.phabricator.com/D11378
Summary: Fixes T6957. If / when a policy object is destroyed, access to an object that uses that policy object is denied.
Test Plan: looked around in the code to fail confident enough to write the summary above
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6957
Differential Revision: https://secure.phabricator.com/D11380
Summary: We could still miss this if the policy had never been customized and we returned early after one of the other checks.
Test Plan:
Works great on instances now.
{F267067}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11374
Summary: Third time lucky... the filename should match the class name now.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11362
Summary: Fast commit. Also forgot to make the config override the existing policy. I *think* this is the right spot and we're good? Ref T6947.
Test Plan: viewed the application settings page for people application and saw the correct overrode setting.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6947
Differential Revision: https://secure.phabricator.com/D11373
Summary: Ref T5833. This doesn't do anything yet, but will allow new instances to automaticaly bind to an open database without anything too hacky.
Test Plan:
Created a service of this type.
{F267059}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D11372
Summary: Fixes T6947
Test Plan:
locked people.create.user and noted the UI only showed a link to the existing policy with no way to edit it.
tried to set the config to all the various bad things and saw helpful error messages telling me what I did wrong.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6947
Differential Revision: https://secure.phabricator.com/D11358
Summary: Not sure this is obvious enough, but maybe future apps will use as well?
Test Plan: test a project with and without a workboard
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11371
Summary: Ref T6947.
Test Plan: made the setting say only admin user a and noted admin user b lost access
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4137, T6947
Differential Revision: https://secure.phabricator.com/D11357
Summary: Reduces visual duplication in a few places.
Test Plan: Review pages in sandbox, see image removed.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11354
Summary: Ref T6947.
Test Plan: toggled setting in application settings and changes stuck. set policy to admin user a only and could not add a provider as a admin user b.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6947
Differential Revision: https://secure.phabricator.com/D11356
Summary: Fixes T6895, When viewing comment edit history, user should not see a dropdown for each comment edit transaction.
Test Plan: Edit task comment, view comment edit history, comment transactions should not provide a dropdown with action items.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6895
Differential Revision: https://secure.phabricator.com/D11355
Summary: Fixes T6917, swallow exception when saving blocking tasks with no changes
Test Plan: Open task, "Edit Blocking Tasks", save without changing, dialog should close with no exception
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6917
Differential Revision: https://secure.phabricator.com/D11353
Summary:
Fixes T5352. This is very useful for finding things that should be easy to do ("not blocked") as well as things that are important to do ("blocking"). I have wanted to check out the latter case in our installation, though no promises on what I would end up actually doing from that search result list. =D
I also think supporting something like T6638 is reasonable but the UI seems trickier to me; its some sort of task tokenizer, which I don't think we've done before?
Test Plan: toggled various search options and got reasonable results. When i clicked conflicting things like "blocking" and "not blocking" verified it was like I had not clicked anything at all.
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5352
Differential Revision: https://secure.phabricator.com/D11306
Summary: Adds more user friendly copy to the result list
Test Plan: Test on a project with and without tasks.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11352
Summary: In my use case, I have `notification.client-uri` set to `https://phabricator.example.com/ws/` (which routes to `nginx`) but I need `aphlict` to listen to port `22280`.
Test Plan: Tested in our install.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11337
Summary:
A refresh of Projects including a new navigations UI.
- New Navigation UI.
- Auto switch default page if Workboard has been initialized
- Move Feed to it's own page
- Increase 'tasks' on Project Home to 50 over 10
- Fix various display bugs on Workboards
- Remove 'crumbs' from Project portal (unneeded).
Test Plan:
- clicked a link for a project with no workboard and saw the profile
- clicked a link for a project with a workboard and saw the workboard
- navigated around the various edit pages, inspecting links and making sure things linked back to the new profile uri
{F266460}
{F266461}
{F266462}
{F266463}
{F266464}
Reviewers: epriestley, btrahan
Reviewed By: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11272
Summary:
Yeahhhhhhhh....
- Open a "stream", not a "steam".
- Make error easier for users to understand.
- Write to the log in debug mode so the issue is more apparent.
Test Plan:
- Started server with bad permissions, got usable error message.
- Started server with good permissions, got logfile.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11339
Summary: Ref T5752, moves mobile action menus to the object box instead of crumbs.
Test Plan: View action menus at tablet, desktop, and mobile break points. Verify clicking buttons works as expected opening menu.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5752
Differential Revision: https://secure.phabricator.com/D11340
Summary: Fixes T6937. We weren't passing required parameters.
Test Plan: Followed repro steps in task.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6937
Differential Revision: https://secure.phabricator.com/D11346
Summary:
Fixes T6932. Fixes some issues from D11303.
- When claiming a task, if it was previously unassigned, we would try to CC `null`.
- When claiming a task, if the current owner was already CC'd, the viewer would incorrectly be warned about all subscribers being CC'd.
Test Plan:
- Claimed an unclaimed task.
- Claimed a task with owner CC'd.
Reviewers: btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T6932
Differential Revision: https://secure.phabricator.com/D11336
