1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 00:02:41 +01:00
Commit graph

12 commits

Author SHA1 Message Date
epriestley
5db26c1b3a Synchronize follower data with parent tasks in Asana bridge
Summary: Ref T2852. Setting followers (like CCs) is a separate API call, but we don't need to do anything complicated.

Test Plan: Synchronized revisions and verified the parent task got followers.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6308
2013-06-25 16:35:55 -07:00
epriestley
816b90a0a1 Try to act with the correct voice in Asana sync
Summary:
Ref T2852.

Before trying related users, try using the feed story's actor. This is the most correct voice to act in.

Test Plan: Ran `feed/republish`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6305
2013-06-25 16:35:19 -07:00
epriestley
c99ebe8402 Synchronize Asana task and subtask states accurately
Summary:
Ref T2852.

The parent task is open unless the revision is in the states "closed" or "abandoned". If it's in "needs review", it remains open. This last bit is slightly unlike Differential, but consistent with the Google Doc and generally seems like a better fit. There's no way to put the task in a "Waiting on Others" state in Asana like we can in Differential.

The subtasks are closed unless the revision is in the state "needs review". This is generally consistent with Differential.

Test Plan:
Made a series of changes to a revision and synchronized it repeatedly:

  - requested changes
  - commandeered
  - requested review
  - abandoned

Verified task and subtasks synchronized states correctly in Asana.

{F47554}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6304
2013-06-25 16:34:59 -07:00
epriestley
302da70e72 Synchronize review request state to Asana
Summary:
Ref T2852. Depends on D6302. This now creates, destroys, and synchronizes subtasks.

  - After finishing the parent task stuff, we pull a list of all known subtasks.
  - We load all those subtasks.
  - If we fail to load any, we delete their objects and edges on the Phabricator side.
  - Of the remaining subtasks, we find subtasks for users who aren't related to the object any more and delete them in Asana and locally (for example, if alincoln is removed as a reviewer, we delete his subtask).
  - For all the related users, we either synchronize their existing task or create a new one for them.
  - Then we write edges for any new tasks we added.

This doesn't handle a few weird edge cases in any specific way:

  - If a subtask is moved under a different parent, we ignore it.
  - If a new subtask is created that we don't know about, we ignore it.
  - If a subtask we know about is deleted, we just respawn it. This is consistent with "DON'T EDIT THESE". You can force sync to stop by deleting the parent.

Addititionally:

  - Make the "don't edit" warning more compelling and visceral.

Test Plan:
  - Kind of ran it a bit.
  - There are like 3,000 edge cases here so this is hard to test exhaustively.
  - Forced a few of the edge cases to happen.
  - Nothing seems immediately broken in an obvious way?

{F47551}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6303
2013-06-25 16:33:46 -07:00
epriestley
5a6044dbaa Initial Asana sync for Differential
Summary:
Ref T2852. This is highly incomplete but seems structurally sound. Some additional context is available in the Google doc.

  - Add a workspace ID configuration. Without it, nothing else activates.
  - Add a worker which reacts to feed stories.
  - Feed stories about things which aren't Differential objects are ignored.
  - We load the revision, or fail permanently if we can't.
  - We get all the related user PHIDs (author, reviewers, CCs).
  - We check if any of them have linked Asana accounts, or fail permanently if they don't.
  - We check for an "ASANATASK" edge from the revision.
    - If we do not find one, we create a new task.
    - If we do find one, we load the task.
      - If we succeed, we check the chronological key of the most recent synchronized feed story ("cursor").
        - If this story is the same or newer, we update the task to synchronize it to the current state of the revision.
      - If we fail to load the task, we fail permanently ("asana task has been deleted").
  - We then publish the actual story text to the task.

Not in yet:

  - Updating followers requires separate API calls which we don't do yet.
  - No subtasks yet.
  - No sync of open/closed state.

Test Plan: {F47546}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6302
2013-06-25 16:33:16 -07:00
epriestley
9e82c01a8a Pull some constants out of the Asana bridge
Summary: Ref T2852. Reduce the number of magical strings in use, and prepare the Asana bridge for eventual workspace/project support (a little bit).

Test Plan: Verified enriched links still work properly.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6301
2013-06-25 16:32:45 -07:00
epriestley
4ca38612f2 Add DoorkeeperExternalObjectQuery, expose more ref/import APIs
Summary:
Ref T2852.

  - Broadly, we support "I have a Ref, I need a PHID" well but not "I have a PHID, I need a Ref".
  - Add DoorkeeperExternalObjectQuery, and use it to query ExternalObjects.
  - Allow external objects to be imported by their internal PHIDs. Basically, if we have an edge pointing at an ExternalObject, we can say "load all the data about this" from just the PHID and have it hit all the same code.
  - Allow construction of Refs from ExternalObjects. This makes the "I have a PHID, I need a Ref" easier.

Test Plan:
  - Verified Asana links still enrich properly at display time.
  - Used in future revision.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6294
2013-06-25 16:30:44 -07:00
epriestley
f8ed6422f8 Provide an auto-refresh mechanism for OAuth providers to deliver fresh tokens
Summary:
Ref T2852. Give OAuth providers a formal method so you can ask them for tokens; they issue a refresh request if necessary.

We could automatically refresh these tokens in daemons as they near expiry to improve performance; refreshes are blocking in-process round trip requests. If we do this for all tokens, it's a lot of requests (say, 20k users * 2 auth mechanisms * 1-hour tokens ~= a million requests a day). We could do it selectively for tokens that are actually in use (i.e., if we refresh a token in response to a user request, we keep refreshing it for 24 hours automatically). For now, I'm not pursuing any of this.

If we fail to refresh a token, we don't have a great way to communicate it to the user right now. The remedy is "log out and log in again", but there's no way for them to figure this out. The major issue is that a lot of OAuth integrations should not throw if they fail, or can't reasonably be rasied to the user (e.g., activity in daemons, loading profile pictures, enriching links, etc). For now, this shouldn't really happen. In future diffs, I plan to make the "External Accounts" settings page provide some information about tokens again, and possibly push some flag to accounts like "you should refresh your X link", but we'll see if issues crop up.

Test Plan: Used `bin/auth refresh` to verify refreshes. I'll wait an hour and reload a page with an Asana link to verify the auto-refresh part.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6280
2013-06-24 15:56:01 -07:00
epriestley
c94ef134e4 Add bin/auth refresh for debugging OAuth token refresh issues
Summary: Ref T2852. Provide a script for inspecting/debugging OAuth token refresh.

Test Plan: Ran `bin/auth refresh` with various arguments, saw token refreshes.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6276
2013-06-24 15:55:41 -07:00
epriestley
b22e52e40c Add remarkup support for Asana URIs
Summary:
Ref T2852. Primarily, this expands API access to Asana. As a user-visible effect, it links Asana tasks in Remarkup.

When a user enters an Asana URI, we register an onload behavior to make an Ajax call for the lookup. This respects privacy imposed by the API without creating a significant performance impact.

Test Plan: {F47183}

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6274
2013-06-24 15:55:08 -07:00
epriestley
e723b7e119 Add DoorkeeperObjectRef, DoorkeeperBridge, DoorkeeperBridgeAsana
Summary:
  - `DoorkeeperObjectRef` is a convenience object to keep track of `<applicationType, applicationDomain, objectType, objectID>` tuples.
  - `DoorkeeperBridge` provides pull/push between Phabricator and external systems.
  - `DoorkeeperBridgeAsana` is a bridge to Asana.

Test Plan:
Ran this snippet and got a task from Asana:

{P871}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6273
2013-06-24 15:54:54 -07:00
epriestley
f54a5d8087 Add DoorkeeperExternalObject
Summary:
Ref T2852. This table holds data about external objects and allows us to write edges to them.

Objects are identified with an `<applicationType, applicationDomain, objectType, objectID>` tuple. For example, Asana tasks will be, e.g., `<asana, asana.com, asana:task, 93829279873>` or similar.

Test Plan: Ran storage upgrade.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6271
2013-06-24 15:54:36 -07:00