Page MenuHomePhabricator

Fallback for incorrect default branch in Diffusion could be nicer
Closed, ResolvedPublic

Description

When we don't have a known default branch in Diffusion, all features shut down (tabs, branches, etc). We could handle this better.

Revisions and Commits

Event Timeline

epriestley renamed this task from Diffusion doesn't render tabs if master isn't present in git to Diffusion doesn't render tabs if a repository has commits and a "Default Branch" set to a branch which does not exist.Jul 21 2017, 5:38 PM

I believe the issue here is:

  • If a repository has no commits, we show an "empty repository" state which (presumably?) has proper navigation.
  • If a repository has commits, and has "Default Branch" set to a branch which does not exist, we (presumably?) do not show navigation, but should.

A simpler reproduction case is likely:

  • For any normal repository, set "Default Branch" to "laknsdflandslfafdalksnanfslfsa" (any branch name which does not exist).
  • Save changes.
  • View repository main page.

ok thanks for the nice and easy repo steps. I'll have something after lunch, got appt at 11

The reason I took navigation away is we throw exceptions instead of error messages when master isn't present (or have no commits yet). Do you remember the reasoning there (using Exception)?

Before we didn't render the table, so the link to visit said table as full screen, doesn't exist. I think I need... better fallback here.

Where is the exception coming from?

(The reasoning is probably just that we always throw exceptions by default when we hit errors and don't have a reason not to throw an exception.)

I'm attempting fixing this right / long-term. I think I'm ok right now.

I think the part that is tripping me up is this never worked, that is, if we had a bad default branch, basically that's all we rendered. We didn't also render a list of good branches.

Like, it's not a regression from the Diffusion re-work.

This is what it did in March 2016: T3607#163684

I considered that "reasonable" at the time because it was enough to fix it yourself: you can click the "Edit Repository" link and change the default branch to one that actually exists.

I'm not sure what the UI looks like now. If you can get to "Manage Repository" I don't think this is really a bug. We could probably improve the behavior, but this is basically a misconfiguration (you've set the default branch to a branch which does not exist) and if you can "Manage" you can go fix it.

That is, my expectation when you have set "Default Branch" to "mmmmmmmmmmmmm" and visit the repository page is just:

  • There is an error message which makes it reasonably clear what the problem is.
  • There is some kind of link to let you fix it.

If both those points are true, this isn't a regression and I don't think the behavior is a bug. I think you've misconfigured the repository, we're telling you what's wrong, and we're giving you the tools to fix it. That seems entirely reasonable to me.

I will ship my branch selector dropdown and make sure it works without a good default branch.

chad renamed this task from Diffusion doesn't render tabs if a repository has commits and a "Default Branch" set to a branch which does not exist to Fallback for incorrect default branch in Diffusion could be nicer.Jul 21 2017, 10:28 PM
chad reopened this task as Open.
chad updated the task description. (Show Details)

I don't know, I'm confused today. I'll see if I can make this cleaner at least. Somehow. We depend a lot on having a default branch though, like picking commit crumbs.

hi,

The problem here is the default for the default branch, when the user didn't set it explicitly. In a user perspective it's then hard to know there's this option at all. Some link to set the default branch from the error could be more clear, but I don't think that's a common in phab to do that.

The branch selector may fix that though, i guess that it will somehow force selection of the first element in the selector ? or it will at least be there for the user to explore the repo from the ui.

Thanks anyway for the discussion, btw I really like where the new diffusion ui is going :)

I'll add a link too, it's reasonable.