Page MenuHomePhabricator
Create Task
Maniphest T12870

Configuration check for "maniphest.priorities" does not validate that keywords are unique across priorities
Closed, ResolvedPublic
Actions

Description

I'm using the current master, and the following config:

	{
  "100": {
    "name": "Unbreak Now!",
    "short": "Unbreak!",
    "color": "pink",
    "keywords": [
      "unbreak"
    ]
  },
  "90": {
    "name": "Needs Triage",
    "short": "Triage",
    "color": "violet",
    "keywords": [
      "triage"
    ]
  },
  "80": {
    "name": "High",
    "short": "High",
    "color": "red",
    "keywords": [
      "high"
    ]
  },
  "70": {
    "name": "Higher than normal",
    "short": "Higher than normal",
    "color": "orange",
    "keywords": [
      "high"
    ]
  },
  "50": {
    "name": "Normal",
    "short": "Normal",
    "color": "yellow",
    "keywords": [
      "normal"
    ]
  },
  "25": {
    "name": "Lower than normal",
    "short": "Lower than normal",
    "color": "green",
    "keywords": [
      "low"
    ]
  },
  "1": {
    "name": "Low",
    "short": "Low",
    "color": "sky",
    "keywords": [
      "low"
    ]
  },
  "0": {
    "name": "Wishlist",
    "short": "Wishlist",
    "color": "skyblue",
    "keywords": [
      "wishlist"
    ]
  }
}

There are two issues:

  • a) I'm not seeing all prios:

prio2.PNG (362×503 px, 10 KB)

  • b) If I'm choosing one prio, phab shows another (not always):

prio1.PNG (410×513 px, 11 KB)

This happened after an update, I did not change the config for weeks.

Revisions and Commits

rP Phabricator
D18165rP6984d239b02b Convert Maniphest custom config to new config types

Related Objects
Search...

Event Timeline

Luke081515.2 created this task.Jun 23 2017, 5:30 PM

I was able to track this down: This is happening since rP8008ade9af46: Use keywords instead of ints to update task priority in ManiphestEditEngine.

Luke081515.2 added a comment.Jun 23 2017, 5:34 PM

This looks like the update changed the behaviour, not sure if this is intended, but it broke configs, that worked before. I currently fixed this by chaning my config to:

{
  "100": {
    "name": "Unbreak Now!",
    "short": "Unbreak!",
    "color": "pink",
    "keywords": [
      "unbreak"
    ]
  },
  "90": {
    "name": "Needs Triage",
    "short": "Triage",
    "color": "violet",
    "keywords": [
      "triage"
    ]
  },
  "80": {
    "name": "High",
    "short": "High",
    "color": "red",
    "keywords": [
      "high"
    ]
  },
  "70": {
    "name": "Higher than normal",
    "short": "Higher than normal",
    "color": "orange",
    "keywords": [
      "high-than-normal"
    ]
  },
  "50": {
    "name": "Normal",
    "short": "Normal",
    "color": "yellow",
    "keywords": [
      "normal"
    ]
  },
  "25": {
    "name": "Lower than normal",
    "short": "Lower than normal",
    "color": "green",
    "keywords": [
      "lower-than-normal"
    ]
  },
  "1": {
    "name": "Low",
    "short": "Low",
    "color": "sky",
    "keywords": [
      "low"
    ]
  },
  "0": {
    "name": "Wishlist",
    "short": "Wishlist",
    "color": "skyblue",
    "keywords": [
      "wishlist"
    ]
  }
}
Luke081515.2 added a project: Maniphest.Jun 23 2017, 5:36 PM
Herald added a subscriber: eadler. · View Herald TranscriptJun 23 2017, 5:36 PM
epriestley added a subscriber: epriestley.Jun 23 2017, 6:19 PM

Specifically, the issue is that multiple different priorities used the same keywords ("low", "high").

I'll add a specific check for this, but probably after fixing T12845.

The docs for this option are also a little misaligned:

Screen Shot 2017-06-23 at 11.19.10 AM.png (57×1 px, 31 KB)

epriestley renamed this task from Prioritys are wrong to Configuration check for "maniphest.priorities" does not validate that keywords are unique across priorities.Jun 23 2017, 6:20 PM
epriestley added a parent task: T12845: Convert remaining ancient configuration type classes to modern modular type classes.Jun 26 2017, 2:04 PM
amckinley added a subscriber: amckinley.Jun 26 2017, 3:34 PM
epriestley added a revision: D18165: Convert Maniphest custom config to new config types.Jun 27 2017, 3:15 PM
epriestley closed this task as Resolved by committing rP6984d239b02b: Convert Maniphest custom config to new config types.Jun 27 2017, 7:35 PM
epriestley added a commit: rP6984d239b02b: Convert Maniphest custom config to new config types.
epriestley added a comment.Jun 27 2017, 7:37 PM

At HEAD of master:

  • You will now be warned about this error specifically, using tailored language which points at the exact problem.
  • The documentation clearly describes this requirement.
  • The documentation got its missing line break back.

This will promote to stable in a few days. See T12845 for some followups. Thanks for the report!

Phabricator · Built by Phacility