Fix Passphrase Credential dialog
ClosedPublic
Actions

Authored by epriestley on Apr 13 2016, 12:32 AM.

Details

Reviewers

chad

Maniphest Tasks

T10772: Can't create new credentials for remote repository

Commits

rPab63b1b2ae34: (stable) Fix Passphrase Credential dialog
rPabf37aa97947: Fix Passphrase Credential dialog

Summary

Fixes T10772, not sure why this fails, but reverting the code back to old dialog call works.

Test Plan

Try to add a new credential when importing a repository.
Also created a new credential normally, via Passphrase.
Also edited a credential.

Diff Detail

Repository

rP Phabricator

Branch

pphrase1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 11681
Build 14623: Run Core Tests
Build 14622: arc lint + arc unit

Event Timeline

chad updated this revision to Diff 37808.Apr 13 2016, 12:32 AM

chad retitled this revision from to Fix Passphrase Credential dialog.

chad updated this object.

chad edited the test plan for this revision. (Show Details)

chad added a reviewer: epriestley.

chad added a task: T10772: Can't create new credentials for remote repository.

Herald added a subscriber: Korvin. · View Herald TranscriptApr 13 2016, 12:32 AM

Let me see if I can figure out what's going on here, these are supposed to be equivalent.

did you die?

which boss are you on?

[questioning intensifies]

I was writing 1 Big Text

boss = I took a nap

T.T

I'm trying to live vicariously. I'll never play a game like Dark Souls. :( I am casual gamer dad now.

I think I have a sliiightly cleaner-ish fix for this, one sec...

Issue is that newDialog() sets an explicit URI, while new AphrontDialogView() does not.

The explicit URI drops GET parameters. It's sort of good that we do this so that everything is more explicit, this is kind of a bug with the existing form.

Doing it the implicit way could run into other issues with GET parameters conflicting with other GET parameters in unusual cases, which would be more weird/harder to figure out than this.

Put "type" parameter in the form explicitly.

epriestley edited the test plan for this revision. (Show Details)Apr 13 2016, 2:31 AM

chad accepted this revision.Apr 13 2016, 3:09 AM

chad edited edge metadata.

This revision is now accepted and ready to land.Apr 13 2016, 3:09 AM

Closed by commit rPabf37aa97947: Fix Passphrase Credential dialog (authored by chad, committed by epriestley). · Explain WhyApr 13 2016, 11:36 AM

This revision was automatically updated to reflect the committed changes.