Page MenuHomePhorge
Differential D25091

Fix information page about Read-only status
ClosedPublic
Actions

Authored by valerio.bozzolan on Mar 24 2023, 08:41.
Tags
None
Referenced Files
F4116099: D25091.1746960058.diff
Sat, May 10, 10:40
F3982719: D25091.1746558234.diff
Mon, May 5, 19:03
F3968779: D25091.1746520284.diff
Mon, May 5, 08:31
F3968777: D25091.1746520248.diff
Mon, May 5, 08:30
F3968774: D25091.1746520213.diff
Mon, May 5, 08:30
F3945687: D25091.1746445251.diff
Sun, May 4, 11:40
F3927582: D25091.1746388677.diff
Sat, May 3, 19:57
F3777002: D25091.1745869174.diff
Sun, Apr 27, 19:39
View All 66 Files
Subscribers
Cigaryno
Matthew
speck
tobiaswiese

Details

Reviewers
avivey
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15185: Crash on "Readonly" Config page: call to undefined method PlatformSymbols::getPlatformServerSymbol()
Commits
rP9bc3c16b6e75: Fix information page about Read-only status
Summary

This change fixes the following page on your Phorge/Phabricator:

/readonly/config/

If you visit that page (with or without read-only mode), the
following crash will happen:

Call to undefined method PlatformSymbols::getPlatformServerSymbol()

Fixes T15185

Probably the regression was introduced in this point:

rP4d3f0dc7c727e6fdc0e8cba0e8e8f2b8ef1bd16c

This change also renamed "administrator" to "Administrator" in the messages
from that specific page, both because I have great respect for Administrators
and also because that is how this word is spelled in most other places in Phorge.

Test Plan
  • Visit the page /readonly/config/
  • It does not crash anymore and you can see that beautiful message that - it seems - no person in the world has ever seen since a whole year. Take a good look at this page: it is a precious treasure.

Diff Detail

Repository
rP Phorge
Branch
config-readonly
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 168
Build 168: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Mar 24 2023, 08:41
Owners added a reviewer: O1: Blessed Committers.Mar 24 2023, 08:41
Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptMar 24 2023, 08:41
valerio.bozzolan requested review of this revision.Mar 24 2023, 08:41
Harbormaster completed remote builds in B168: Diff 380.Mar 24 2023, 08:41
valerio.bozzolan edited the summary of this revision. (Show Details)Mar 24 2023, 08:44
valerio.bozzolan mentioned this in D25090: Fix information page about Read-only status.Mar 24 2023, 19:06
valerio.bozzolan mentioned this in Z1: Phorge.Mar 24 2023, 22:56
avivey accepted this revision.Mar 25 2023, 09:41
avivey edited the summary of this revision. (Show Details)
avivey added a task: T15185: Crash on "Readonly" Config page: call to undefined method PlatformSymbols::getPlatformServerSymbol().
This revision is now accepted and ready to land.Mar 25 2023, 09:42
valerio.bozzolan added a comment.Mar 25 2023, 09:44

Thank you for your review

Note that I use to mention the Task URL in the commit body since I like to have a human description accessible from your local git, or any other mirror (local, or GitHub, etc.); and, have Diffusion actions in the footer with short "Ref .." stuff for machines.

If you want, I can avoid my behavior, even if I think it's reasonable.

Closed by commit rP9bc3c16b6e75: Fix information page about Read-only status. · Explain WhyMar 25 2023, 11:06
This revision was automatically updated to reflect the committed changes.
valerio.bozzolan added a commit: rP9bc3c16b6e75: Fix information page about Read-only status.
valerio.bozzolan mentioned this in 2023 Week 2.Mar 25 2023, 11:07
avivey added a comment.Mar 25 2023, 12:32

Since the context of these changes is all localized here, I think it's reasonable to use the short names (T1234), because we expect most users will see it in the context of this site. I just find the full addresses to be harder to read (I need to stop and actually read the URI to understand if this is an "internal" or "external" link).

I don't mind if you want to have both, I just think it's redundant.

avivey mentioned this in 2023 Week 17.Apr 25 2023, 06:54

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)Packages
resources/
celerity/
100%
Loading...
O1: Blessed Committers
src/
applications/
system/
controller/
6 lines1%
Loading...
O1: Blessed Committers

Diff 380

View Options

resources/celerity/map.php

Show All 37 Lines public function setIcon($icon) {
$this->icon = $icon; $this->icon = $icon;
return $this; return $this;
} }
public function getIcon() { public function getIcon() {
return $this->icon; return $this->icon;
} }
/**
* Set the text of the tag
*
* This is usually the 'name' key of your subtype map.
* Sometime this is an uppercase text like 'BUG' for a 'bug' subtype name.
*
* @param string|null $text
* @return self
*/
public function setTagText($text) { public function setTagText($text) {
$this->tagText = $text; $this->tagText = $text;
return $this; return $this;
} }
/**
* Get the text of the tag
*
* @see PhabricatorEditEngineSubtype::setTagText()
*
* @return string|null
*/
public function getTagText() { public function getTagText() {
return $this->tagText; return $this->tagText;
} }
public function setColor($color) { public function setColor($color) {
$this->color = $color; $this->color = $color;
return $this; return $this;
} }
Show All 25 Lines public function setMutations($mutations) {
return $this; return $this;
} }
public function getMutations() { public function getMutations() {
return $this->mutations; return $this->mutations;
} }
public function hasTagView() { public function hasTagView() {
return (bool)strlen($this->getTagText()); return phutil_nonempty_string($this->getTagText());
valerio.bozzolanUnsubmitted
Done
Inline Actions

✅ The only usage of setTagText() seems to be in PhabricatorEditEngineSubtype and the input domain is just a string coming from phutil_utf8_strtoupper() (like "BUG") or null (default). The phutil_nonempty_string() will report alien values and this is OK.

Also, the method was returning a bool, and it still returns a bool.

valerio.bozzolan: ✅ The only usage of `setTagText()` seems to be in `PhabricatorEditEngineSubtype` and the input…
} }
public function newTagView() { public function newTagView() {
$view = id(new PHUITagView()) $view = id(new PHUITagView())
->setType(PHUITagView::TYPE_OUTLINE) ->setType(PHUITagView::TYPE_OUTLINE)
->setName($this->getTagText()); ->setName($this->getTagText());
$color = $this->getColor(); $color = $this->getColor();
▲ Show 20 Lines • Show All 217 Lines • Show Last 20 Lines
View Options

src/applications/system/controller/PhabricatorSystemReadOnlyController.php

Show All 37 Lines public function setIcon($icon) {
$this->icon = $icon; $this->icon = $icon;
return $this; return $this;
} }
public function getIcon() { public function getIcon() {
return $this->icon; return $this->icon;
} }
/**
* Set the text of the tag
*
* This is usually the 'name' key of your subtype map.
* Sometime this is an uppercase text like 'BUG' for a 'bug' subtype name.
*
* @param string|null $text
* @return self
*/
public function setTagText($text) { public function setTagText($text) {
$this->tagText = $text; $this->tagText = $text;
return $this; return $this;
} }
/**
* Get the text of the tag
*
* @see PhabricatorEditEngineSubtype::setTagText()
*
* @return string|null
*/
public function getTagText() { public function getTagText() {
return $this->tagText; return $this->tagText;
} }
public function setColor($color) { public function setColor($color) {
$this->color = $color; $this->color = $color;
return $this; return $this;
} }
Show All 25 Lines public function setMutations($mutations) {
return $this; return $this;
} }
public function getMutations() { public function getMutations() {
return $this->mutations; return $this->mutations;
} }
public function hasTagView() { public function hasTagView() {
return (bool)strlen($this->getTagText()); return phutil_nonempty_string($this->getTagText());
valerio.bozzolanUnsubmitted
Done
Inline Actions

✅ The only usage of setTagText() seems to be in PhabricatorEditEngineSubtype and the input domain is just a string coming from phutil_utf8_strtoupper() (like "BUG") or null (default). The phutil_nonempty_string() will report alien values and this is OK.

Also, the method was returning a bool, and it still returns a bool.

valerio.bozzolan: ✅ The only usage of `setTagText()` seems to be in `PhabricatorEditEngineSubtype` and the input…
} }
public function newTagView() { public function newTagView() {
$view = id(new PHUITagView()) $view = id(new PHUITagView())
->setType(PHUITagView::TYPE_OUTLINE) ->setType(PHUITagView::TYPE_OUTLINE)
->setName($this->getTagText()); ->setName($this->getTagText());
$color = $this->getColor(); $color = $this->getColor();
▲ Show 20 Lines • Show All 217 Lines • Show Last 20 Lines
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0