Summary:
Ref T13658. This adds a simple expression evaluator to Remarkup and supports platform name expressions. The syntax is:
```
${{{strings.platform.server.name}}}
```
Note that this won't work inside code blocks (or literal blocks, or other block-level literal elements) right now, although it could be made to selectively (the ".path" expressions might be useful in documentation codeblocks).
Test Plan: {F9391006}
Reviewers: cspeckmim
Reviewed By: cspeckmim
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21713
Summary: Ref T13072. This exception is now raised by all of the message-sending code. Pretty straight find/replace.
Test Plan: Grepped for old class name, no hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21699
Summary: Ref T13072. Make large Conduit doc pages a bit more navigable. This prepares for updating "harbormaster.sendmessage" to support sending messages to builds.
Test Plan: Viewed various Conduit API documentation pages, clicked links.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21696
Summary: Ref T13072. These don't do anything useful yet, but get the skeletons in.
Test Plan: Loaded documentation pages without fataling.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21695
Summary: Ref T13072. Trivially convert this into a modular transaction type.
Test Plan: Issued commands to a buildable.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21694
Summary: Ref T13072. Update the last few constant references to this class and remove it.
Test Plan: Grepped for "HarbormasterBuildCommand", got no hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21692
Summary:
Ref T13072. Further modularize build messages by applying each one in a separate transaction type.
This makes it easier to add new types of messages (although I have no particular plans to do this, offhand) and reduces the amount of switch-boilerplate.
This will probably also simplify validating "harbormaster.sendmessage".
Test Plan:
- Applied all commands.
- Ran migration, saw transactions render properly
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21690
Summary: Ref T13072. Update "HarbormasterBuild" to use modern modular transactions.
Test Plan:
- Aborted, restarted, paused, and resumed a build.
- Used `bin/harbormaster restart`.
- Grepped for use of old "::TYPE_COMMAND" constant, didn't find any hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21689
Summary:
Ref T13072. These two similar tables don't make sense to keep separate. Instead, make Build a valid receiver for BuildMessage objects.
These tables are practically the same, so this is straightforward: just copy the rows in and then drop the old table.
(This table was trivial and ephemeral anyway, so I'm not bothering to do the usual "keep it around for a couple years just in case".)
Test Plan:
- Populated BuildCommand table, ran migration, saw Builds end up in the proper transitional state (e.g., pausing, aborting, restarting) with appropriate queued messages.
- Queued new messages by clicking UI buttons.
- Ran BuildWorkers, saw them process messages and mark them as consumed.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21684
Summary:
With newer versions of Mercurial come newer debug messages which need filtered out.
1. In the scenario of Phabricator observing a hosted Mercurial repository which exists on a server in a multi-user environment it's possible that a repository computes branch cache at a tip revision which is not present. When this happens Mercurial will include in the debug output this information. This message indicates that the cache is going to be re-computed. See https://www.mercurial-scm.org/pipermail/mercurial/2014-June/047239.html.
2. Likely in some version with added or improved support for `pager` the debug info seems to indicate when a pager is being invoked for a command. This seems to print out regularly despite piping the stdout.
3. If the repository on Phabricator ever had the `largefiles` extension enabled then some additional details about "updated patterns" will print out.
Test Plan:
I verified an observed repository's history could be browsed, specifically the history of files which previously resulted in "Undefined offset: 1".
Added a unit test to check the results of `filterMercurialDebugOutput()`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21677
Summary: See T13657. An install has "watcher" packages which should not allow owners to "Force Accept" other packages.
Test Plan:
- Created package A, which I own, on "/", with "Weak" authority.
- Created package B, which I do not own, on "/src".
- Created a revision which touches "/src" and added package B as a reviewer.
- Attempted to accept the revision...
- Before patch: permitted to "Force Accept" for package B.
- After patch: not allowed to "Force Accept" for package B.
- Verified that setting package "A" back to "Strong" authority allows a force-accept for package B.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D21674
Summary: Ref T13614. Provide "bin/repository lock" to temporarily lock repositories for manual maintenance.
Test Plan:
- Read instructions.
- Used `bin/repository lock` according to the instructions.
- Saw Storage tab in Diffusion report lock held during maintenance, released after it completes.
- Saw "maintenance" push log generated and repository version bump.
- Tried to lock some invalid repositories.
Maniphest Tasks: T13614
Differential Revision: https://secure.phabricator.com/D21671
Summary:
Ref T9764. These "star" icons are unclear, inconsistent, and not friendly to colorblind users.
They date from a time long ago when the product didn't have icons.
Modernize them and make them more consistent with the similar statuses in Harbormaster.
Test Plan:
{F8545690}
{F8545691}
{F8545692}
Maniphest Tasks: T9764
Differential Revision: https://secure.phabricator.com/D21639
Summary:
Ref T13641. Add a "status" property with most of the relevant support code.
This currently has no impact on use of the device or bindings by Diffusion or Drydock: they ignore the status of devices bound to services.
Test Plan:
- Created a new device.
- Changed the status of a device via web and API.
- Queried for devices via API.
- Searched for active and disabled devices.
- Viewed UI in list view, detail view.
- Used typeahead to add a new binding to an interface on a disabled device, got disabled hint in typeahead UI.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21627
Summary:
Ref T13639. Move operations related to updating the "AffectedPath" index to a dedicated class.
This change has no functional effect and only moves code.
Test Plan: Used an external script to rebuild every revision index; destroyed a revision with `bin/remove destroy`.
Maniphest Tasks: T13639
Differential Revision: https://secure.phabricator.com/D21616
Summary:
Ref T13639. The "Affected Path" table is currently hard to inspect: there's no UI, and using MySQL just gives you a bunch of IDs.
Add a simple UI and a debug-mode link to it.
Test Plan:
{F8539098}
{F8539099}
Maniphest Tasks: T13639
Differential Revision: https://secure.phabricator.com/D21615
Summary: Ref T13636. Add routing for "/robots.txt", "favicon.ico", and "/status/" on the ShortSite and BlogSite.
Test Plan: Visted all resources (and 404 pages) on Short and Blog sites, and Platform site.
Maniphest Tasks: T13636
Differential Revision: https://secure.phabricator.com/D21607
Summary:
Fixes T12919. Fixes T13636. Prior to this change, some well-known resource paths don't route on sites like ResourceSite.
- `/robots.txt`: Make it route on ResourceSite and just deny the whole site.
- `/favicon.ico`: Make it route on ResourceSite.
- `/status/`: Make it route on ResourceSite.
- 404: Make it render a 404 on ResourceSite.
Test Plan:
- Visited all URIs on ResourceSite, got sensible responses.
- Visited all URIs on main site.
- Visited 404 while logged out, got login page.
Maniphest Tasks: T13636, T12919
Differential Revision: https://secure.phabricator.com/D21606
Summary:
Ref T13635. Currently, the JSON DocumentEngine uses "phutil_json_decode()", but this can confuse "{}" and "[]".
Be more careful about how the JSON value is decoded, to preserve the distinction.
Test Plan: {F8520479}
Maniphest Tasks: T13635
Differential Revision: https://secure.phabricator.com/D21605
Summary:
Ref T13631. Move "PhabricatorAuditStatusConstants" to a more modern object ("PhabricatorAuditRequestStatus").
Expose the status value via Conduit.
Test Plan:
- Ran `bin/audit delete`.
- Viewed a commit with auditors in the web UI.
- Grepped for affected symbols.
- Called Conduit with the "auditors" attachment, saw auditor statuses.
Maniphest Tasks: T13631
Differential Revision: https://secure.phabricator.com/D21599
Summary: Ref T13631. For now, this only shows the auditor PHID. The current status constants could use some cleanup before they're exposed.
Test Plan: Queried with "auditors" attachment, saw basic auditor information.
Maniphest Tasks: T13631
Differential Revision: https://secure.phabricator.com/D21597
Summary:
Ref T13629.
- Allow files to have custom alt text.
- If a file doesn't have alt text, try to generate a plausible default alt text with the information we have.
Test Plan:
- Viewed image files in DocumentEngine diffs, files, `{Fxxx}` embeds, and lightboxes.
- Saw default alt text in all cases, or custom alt text if provided.
- Set, modified, and removed file alt text. Viewed timeline and feed.
- Pulled alt text with "conduit.search".
Maniphest Tasks: T13629
Differential Revision: https://secure.phabricator.com/D21596
Summary:
Ref T13628. Currently, Differential has a "Commandeer" action, but no way to edit the author otherwise.
This is largely archaic: there is no reason to prevent editing the author, and this makes it difficult to undo mistakes if you commandeer by accident.
Instead, provide a normal "Author" field and a "Foist Upon" action, similar to the "Owner" and "Claim/Assign" fields in Maniphest.
Test Plan:
- Applied author transactions as the old author ("foisted"), the new author ("commandeerd"), and an arbitrary third party ("changed author").
- Tried to unassign author, etc.
- Viewed stories in feed and transaction timeline.
- Saw sensible automatic reviewer changes.
- Used existing "Commandeer" action, which is unchanged.
- Called "transaction.search" and saw reasonable transaction values.
Maniphest Tasks: T13628
Differential Revision: https://secure.phabricator.com/D21591
Summary: T13578
Test Plan:
This method uses the existing transaction. As such, most of the testing focused on the integration between the workflow and transaction. The only change made to the transaction was to allow an omnipotent user to make the change in addition to an admin.
Other than that, I removed the "approved" flag from the user, then ran the command-line utilty until the user was successfully approved.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T13578
Differential Revision: https://secure.phabricator.com/D21587
Summary:
Ref T13627. Currently, global locks always return connections (even external connections) to the connection pool when unlocked.
This code is obviously buggy: `isExternalConnection` is set to false immediately before it is tested. This bug has existed since this code was introduced, in D15792.
- Instead of storing a flag, store the actual connection.
- Don't clear it when unlocking.
- Don't return external connections to the pool.
Test Plan:
- Added a failing test, made it pass.
Maniphest Tasks: T13627
Differential Revision: https://secure.phabricator.com/D21583
Summary:
Ref T13617. When an inline comment is added inside a block of added lines, it currently ends up off-by-one when porting forward.
This is a disagreement between the mapping engine and the display engine about what "offset" means. Choose the simpler of the two interpretations.
Test Plan:
- Created a revision with the diff in T13617.
- Added an inline in the middle of the added block.
- Updated the revision with the same diff.
- Before: inline incorrectly moves up by one line.
- After: inline maps correctly.
Maniphest Tasks: T13617
Differential Revision: https://secure.phabricator.com/D21572
Summary: Ref T13586. In the footsteps of D21563, make Herald rule results more formal and structured to support meaningful exception reporting.
Test Plan:
Ran various Herald rules and viewed transcripts, including rules with recursive dependencies and condition exceptions.
{F8447894}
Maniphest Tasks: T13586
Differential Revision: https://secure.phabricator.com/D21565
Summary: Ref T13586. Lift the behavioral core of "HeraldConditionResult" into a new abstract base "HeraldTranscriptResult", with the intent to introduce a "HeraldRuleResult".
Test Plan:
- Ran Herald rules, reviewed transcripts.
- This change should have no behavioral effect.
Maniphest Tasks: T13586
Differential Revision: https://secure.phabricator.com/D21564
Summary:
Ref T13586. Currently, Herald condition logs encode "pass" or "fail" robustly, "forbidden" through a sort of awkward side channel, and can not properly encode "invalid" or "exception" outcomes.
Structure the condition log so results are represented unambiguously and all possible outcomes (pass, fail, forbidden, invalid, exception) are clearly encoded.
Test Plan:
{F8446102}
{F8446103}
Maniphest Tasks: T13586
Differential Revision: https://secure.phabricator.com/D21563
Summary: Ref T13605. Support selecting a diff's changesets (to get a list of affected file paths) via the API.
Test Plan: Called API with no arguments, diffPHIDs, PHIDs, IDs. Got sensible output.
Maniphest Tasks: T13605
Differential Revision: https://secure.phabricator.com/D21558
Summary:
Ref T13605. Changesets currently have no PHID, which limits their ability to use standard API infrastructure.
Give them a PHID, since there's no reason they don't have one other than their age.
Test Plan:
- Ran migrations, saw PHIDs populated.
- Created new changesets, saw PHIDs.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13605
Differential Revision: https://secure.phabricator.com/D21557
Summary:
Ref T13602. Currently, the policy framework can not execute "test if many users can see one object" particluarly efficiently. This test must be executed more broadly to implement the changes in T13602.
To avoid making this any worse than it already is, lift this block into a wrapper class that has a bulk queue + fetch API and could eventually be optimized.
Test Plan: Viewed a task with an `@mention` of a user without permission to see it in the summary, saw it rendered in a disabled style.
Maniphest Tasks: T13602
Differential Revision: https://secure.phabricator.com/D21546
Summary:
See <https://discourse.phabricator-community.org/t/i-cant-create-almanac-space/4424/>.
Almanac namespaces have never really had property support, but they implemented the interface in the original implementation.
At the time, this had no effect. Later changes integrated properties into the edit flows and broke this no-op integration.
Remove the interface for now. They could be given property support later, but need a bit of support code.
This feature is very rarely used and primarily useful for Phacility instances.
Test Plan: Created new namespaces and edited namespaces, browsed namespace UI.
Differential Revision: https://secure.phabricator.com/D21543
Summary: Ref T13591. Support delaying selected tasks until a later time and bulk-adjustment of task priority.
Test Plan: Ran `bin/worker delay` and `bin/worker priority` to delay and reprioritize tasks. Confirmed outcomes with daemon console.
Maniphest Tasks: T13591
Differential Revision: https://secure.phabricator.com/D21535
Summary: Ref T13585. Provide a minimal but technically functional "harbormaster.step.edit" API method.
Test Plan: Used the web console to modify the URI for a "Make HTTP Request" build step.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13585
Differential Revision: https://secure.phabricator.com/D21489
Summary: Ref T13585. This isn't particularly useful (notably, it does not include custom field values and isn't searchable by build plan PHID) but get the basics into place.
Test Plan: Used the web UI to make API calls, reviewed results.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13585
Differential Revision: https://secure.phabricator.com/D21488
Summary: Ref T13583. To improve support for making it harder to improperly mix data retention policies, allow Herald to act on comment content.
Test Plan:
- Wrote comment content Herald rules in Maniphest and Differential.
- Submitted non-matching comments (no action) and matching comments (Herald action).
- In Differential, triggered rules by submitting non-matching main content and a matching inline comment.
Maniphest Tasks: T13583
Differential Revision: https://secure.phabricator.com/D21479
Summary:
Ref T13552. This is one of two callsites to "diffusion.querycommits". It's an old debugging workflow which I haven't used in years and which is likely obsoleted by identities and other changes.
I believe the root problem here was also ultimately user error (a user has misconfigured their local Git author email as another user).
Test Plan: Grepped for "lookup-users", got no hits.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21444
Summary:
Ref T13552. Commit parsers currently invoke a special mode of "diffusion.querycommits", which is an older frozen method.
The replacement, "diffusion.commit.search", is not really appropriate for low-level access. This mode of having a single method which operates in "cache" or "non-cache" modes also ends up in a lot of unnecessary field shuffling.
Provide "internal.commit.search" as a modern equivalent that returns a "DiffusionCommitRef"-compatible structure.
Test Plan: Executed "internal.commit.search", got sensible low-level commit results.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21443
Summary:
Ref T13570. Fixes T13235. In most cases, we use modern (v4) signatures for almost all AWS API calls, and have for several years.
However, sending email via SES currently uses an older piece of external code which uses the older (v3) signature method.
AWS is retiring v3 signatures on October 1 2020, so this pathway will stop working.
Update the pathway to use `PhutilAWSFuture`, which provides v4 signatures.
T13235 discusses poor error messages from SES. Switching to Futures fixes this for free, as they have more useful error handling.
Test Plan:
- Configured an SES mailer, including the new `region` parameter.
- Used `bin/mail send-test` to send mail via SES.
- Sent invalid mail (from an unverified address); got a more useful error message.
- Grepped for removed external, no hits.
Maniphest Tasks: T13570, T13235
Differential Revision: https://secure.phabricator.com/D21461
Summary:
Ref T13573. Using the browser "Print" feature on pages produces "Thu, Aug 4, 12:22" timestamps which require context to interpret precisely (they don't have a year and don't have a timezone).
Instead, retain these timestamps in "screen" contexts but use "YYYY-MM-DD HH:MM:SS (UTC+X)" timestamps when printing.
Test Plan: Printed Maniphest tasks and other pages in Safari and Chrome using "?__print__=1" and "Print to PDF", saw absolute timestamps after this chagne in the printed documents.
Maniphest Tasks: T13573
Differential Revision: https://secure.phabricator.com/D21451
Summary: Ref T13552. Provide a richer handle/status list item for commit lists.
Test Plan: Viewed commits in various interfaces, saw richer information.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21431
Summary:
Ref T13552. Build the "commit list" elements so that the menu action items collapse under the element on mobile.
Also change the mobile breakpoint to 512px because my Safari window can't go any narrower than 508px. Future changes to responsive design will be more content-aware anyway.
Test Plan: Looked at commits in various interfaces, at desktop and mobile widths.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21430
Summary:
Ref T13552. The current layout doesn't work particularly well on desktops or devices.
We have some device/desktop table layout code, but it isn't generic. We also have property list layout code, but it isn't generic either.
Provide generic layout elements ("Fuel", from "Phabricator UI Layout" to "PHUIL"?) and narrowly specialize their display behavior. Then swap the ListItemView stuff to use it.
Test Plan:
Saw slightly better responsive behavior:
{F7637457}
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21418
Summary: Ref T13552. Remove yet another way to render a list of commits, and unify it with "CommitGraphView".
Test Plan:
- Viewed commit search results.
- Viewed owners package detail page.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21415
Summary:
Ref T13552.
Currently, the "Browse" page shows a snippet of unmerged changes if you're looking at a non-default branch. Remove this for consistency with the simplified main "Browse" page. This is reachable via "Compare".
Update the "Compare" page to use the new "CommitGraphView".
Test Plan:
- Looked at the "Browse" page of "stable".
- Looked at the "Compare" page for "stable vs master".
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21414
Summary:
Ref T13552. This older view mostly duplicates other code and has only two callsites:
- The "Commits" section of user profile pages.
- The "Ambiguous Hash" page when you visit a commit hash page which is an ambiguous prefix of two or more commit hashes.
Replace both with "DiffusionCommitGraphView".
Test Plan:
- Visited profile page, clicked "Commits".
- Visited an ambiguous hash page (`rPbd3c23`).
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21412
Summary:
Ref T13552. Currently, commit lists are sometimes rendered as an object list and sometimes rendered as a table. There are two separate views for table rendering.
Add a fourth view ("list, with a graph") with the eventual intent of unifying all the other views. For now, this only replaces "HistoryListView" -- and needs some more work to really be a convincing replacement.
Test Plan:
- Looked at "History" in Diffusion, saw an ugly view with all the information we want.
- Grepped for "HistoryListView", no hits.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21410
Summary:
Ref T13552. Currently, Diffusion has two effectively identical history views, the "Graph" view and the "History" view.
These arose out of product uncertainty about the importance of the graph, but I think we can just put the graph on the "object item list" view and merge these views.
Test Plan: Looked at repositories in Diffusion, no longer saw a "Graph" tab. Grepped for "graph"-related symbols.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21409
Summary: Ref T13552. This older class has no callers; tag and branch listings were replaced with an "ObjectList" view.
Test Plan: Grepped for "DiffusionTagTableView", got no hits.
Maniphest Tasks: T13552
Differential Revision: https://secure.phabricator.com/D21407
