[PATCH] Add capability for co-maintainers to disown packages
Implements FS#53832 Signed-off-by: Mark Weiman <mark.weiman@markzz.com> --- web/html/pkgbase.php | 3 +++ web/html/pkgdisown.php | 13 ++++++++++--- web/lib/pkgbasefuncs.inc.php | 12 ++++++++++-- web/template/pkgbase_actions.php | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/web/html/pkgbase.php b/web/html/pkgbase.php index 03b0eee..cf9a6c6 100644 --- a/web/html/pkgbase.php +++ b/web/html/pkgbase.php @@ -60,6 +60,9 @@ if (check_token()) { $output = __("The selected packages have not been disowned, check the confirmation checkbox."); $ret = false; } + } elseif (current_action("do_DisownComaintainer")) { + $uid = uid_from_sid($_COOKIE["AURSID"]); + list($ret, $output) = pkgbase_remove_comaintainer($base_id, $uid); } elseif (current_action("do_Vote")) { list($ret, $output) = pkgbase_vote($ids, true); } elseif (current_action("do_UnVote")) { diff --git a/web/html/pkgdisown.php b/web/html/pkgdisown.php index 4b04e85..8da08cf 100644 --- a/web/html/pkgdisown.php +++ b/web/html/pkgdisown.php @@ -7,10 +7,13 @@ include_once("pkgfuncs.inc.php"); html_header(__("Disown Package")); +$action = "do_Disown"; + $maintainer_uids = array(pkgbase_maintainer_uid($base_id)); $comaintainers = pkgbase_get_comaintainers($base_id); +$comaintainer_uids = pkgbase_get_comaintainer_uids(array($base_id)); -if (has_credential(CRED_PKGBASE_DISOWN, $maintainer_uids)): ?> +if (has_credential(CRED_PKGBASE_DISOWN, array_merge($maintainer_uids, $comaintainer_uids))): ?> <div class="box"> <h2><?= __('Disown Package') ?>: <?= htmlspecialchars($pkgbase_name) ?></h2> <p> @@ -23,7 +26,11 @@ if (has_credential(CRED_PKGBASE_DISOWN, $maintainer_uids)): ?> <?php endforeach; ?> </ul> <p> - <?php if (count($comaintainers) > 0 && !has_credential(CRED_PKGBASE_DISOWN)): ?> + + <?php if (in_array(uid_from_sid($_COOKIE["AURSID"]), $comaintainer_uids) && !has_credential(CRED_PKGBASE_DISOWN)): + $action = "do_DisownComaintainer"; ?> + <?= __("By selecting the checkbox, you confirm that you want to no longer be a package co-maintainer.") ?> + <?php elseif (count($comaintainers) > 0 && !has_credential(CRED_PKGBASE_DISOWN)): ?> <?= __('By selecting the checkbox, you confirm that you want to disown the package and transfer ownership to %s%s%s.', '<strong>', $comaintainers[0], '</strong>'); ?> <?php else: ?> @@ -40,7 +47,7 @@ if (has_credential(CRED_PKGBASE_DISOWN, $maintainer_uids)): ?> <?php endif; ?> <p><label class="confirmation"><input type="checkbox" name="confirm" value="1" /> <?= __("Confirm to disown the package") ?></label</p> - <p><input type="submit" class="button" name="do_Disown" value="<?= __("Disown") ?>" /></p> + <p><input type="submit" class="button" name="<?= $action ?>" value="<?= __("Disown") ?>" /></p> </fieldset> </form> </div> diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php index ff1bc90..16f95da 100644 --- a/web/lib/pkgbasefuncs.inc.php +++ b/web/lib/pkgbasefuncs.inc.php @@ -1158,11 +1158,12 @@ function pkgbase_get_comaintainer_uids($base_ids) { * * @param int $base_id The package base ID to update the co-maintainers of * @param array $users Array of co-maintainer user names + * @param boolean $override Override credential check if true * * @return array Tuple of success/failure indicator and error message */ -function pkgbase_set_comaintainers($base_id, $users) { - if (!has_credential(CRED_PKGBASE_EDIT_COMAINTAINERS, array(pkgbase_maintainer_uid($base_id)))) { +function pkgbase_set_comaintainers($base_id, $users, $override=false) { + if (!$override && !has_credential(CRED_PKGBASE_EDIT_COMAINTAINERS, array(pkgbase_maintainer_uid($base_id)))) { return array(false, __("You are not allowed to manage co-maintainers of this package base.")); } @@ -1213,3 +1214,10 @@ function pkgbase_set_comaintainers($base_id, $users) { return array(true, __("The package base co-maintainers have been updated.")); } + +function pkgbase_remove_comaintainer($base_id, $uid) { + $uname = username_from_id($uid); + $names = pkgbase_get_comaintainers($base_id); + $names = array_diff($names, array($uname)); + return pkgbase_set_comaintainers($base_id, $names, true); +} diff --git a/web/template/pkgbase_actions.php b/web/template/pkgbase_actions.php index d3f0592..5eee547 100644 --- a/web/template/pkgbase_actions.php +++ b/web/template/pkgbase_actions.php @@ -41,7 +41,7 @@ <?php if ($uid && $row["MaintainerUID"] === NULL): ?> <li><?= html_action_form($base_uri . 'adopt/', "do_Adopt", __('Adopt Package')) ?></li> - <?php elseif (has_credential(CRED_PKGBASE_DISOWN, array($row["MaintainerUID"]))): ?> + <?php elseif (has_credential(CRED_PKGBASE_DISOWN, array_merge(array($row["MaintainerUID"]), pkgbase_get_comaintainer_uids(array($base_id))))): ?> <li><?= html_action_form($base_uri . 'disown/', "do_Disown", __('Disown Package')) ?></li> <?php endif; ?> </ul> -- 2.16.1
On Tue, 06 Feb 2018 at 03:54:56, Mark Weiman wrote:
Implements FS#53832
Signed-off-by: Mark Weiman <mark.weiman@markzz.com> --- web/html/pkgbase.php | 3 +++ web/html/pkgdisown.php | 13 ++++++++++--- web/lib/pkgbasefuncs.inc.php | 12 ++++++++++-- web/template/pkgbase_actions.php | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-)
[...] @@ -23,7 +26,11 @@ if (has_credential(CRED_PKGBASE_DISOWN, $maintainer_uids)): ?> <?php endforeach; ?> </ul> <p> - <?php if (count($comaintainers) > 0 && !has_credential(CRED_PKGBASE_DISOWN)): ?> + + <?php if (in_array(uid_from_sid($_COOKIE["AURSID"]), $comaintainer_uids) && !has_credential(CRED_PKGBASE_DISOWN)): + $action = "do_DisownComaintainer"; ?> + <?= __("By selecting the checkbox, you confirm that you want to no longer be a package co-maintainer.") ?> + <?php elseif (count($comaintainers) > 0 && !has_credential(CRED_PKGBASE_DISOWN)): ?>
I am not sure whether it is a good idea to use the same button for disowning a package as a maintainer or as a co-maintainer? What happens if a user is both a maintainer and a co-maintainer (and what is the expected behavior)? Anyway, merged this into pu as-is for now; we can still replace it later. Regards, Lukas
On 02/23/2018 12:46 AM, Lukas Fleischer wrote:
I am not sure whether it is a good idea to use the same button for disowning a package as a maintainer or as a co-maintainer? What happens if a user is both a maintainer and a co-maintainer (and what is the expected behavior)?
If it *is* possible to be both, maybe we should fix that instead? :p Anyway, if you click the disown button we assume you want to ditch the package altogether... if you are the maintainer and want to edit the comaintainer list we have a UI for that already. -- Eli Schwartz Bug Wrangler and Trusted User
On Fri, 2018-02-23 at 08:21 -0500, Eli Schwartz wrote:
On 02/23/2018 12:46 AM, Lukas Fleischer wrote:
I am not sure whether it is a good idea to use the same button for disowning a package as a maintainer or as a co-maintainer? What happens if a user is both a maintainer and a co-maintainer (and what is the expected behavior)?
If it *is* possible to be both, maybe we should fix that instead? :p
It is possible and I will submit a patch to fix that this week.
Anyway, if you click the disown button we assume you want to ditch the package altogether... if you are the maintainer and want to edit the comaintainer list we have a UI for that already.
On Mon, 05 Mar 2018 at 07:03:01, Mark Weiman wrote:
On Fri, 2018-02-23 at 08:21 -0500, Eli Schwartz wrote:
On 02/23/2018 12:46 AM, Lukas Fleischer wrote:
I am not sure whether it is a good idea to use the same button for disowning a package as a maintainer or as a co-maintainer? What happens if a user is both a maintainer and a co-maintainer (and what is the expected behavior)?
If it *is* possible to be both, maybe we should fix that instead? :p
It is possible and I will submit a patch to fix that this week.
Anyway, if you click the disown button we assume you want to ditch the package altogether... if you are the maintainer and want to edit the comaintainer list we have a UI for that already.
I am not too sure. The current implementation allows the maintainer to nominate a new maintainer and make himself a co-maintainer (by putting himself at the end of the list of co-maintainers and disowning the package). I guess this functionality will be lost after the "fix"? Regards, Lukas
On Mon, Mar 05, 2018 at 07:39:04AM +0100, Lukas Fleischer wrote:
On Mon, 05 Mar 2018 at 07:03:01, Mark Weiman wrote:
On Fri, 2018-02-23 at 08:21 -0500, Eli Schwartz wrote:
On 02/23/2018 12:46 AM, Lukas Fleischer wrote:
I am not sure whether it is a good idea to use the same button for disowning a package as a maintainer or as a co-maintainer? What happens if a user is both a maintainer and a co-maintainer (and what is the expected behavior)?
If it *is* possible to be both, maybe we should fix that instead? :p
It is possible and I will submit a patch to fix that this week.
Anyway, if you click the disown button we assume you want to ditch the package altogether... if you are the maintainer and want to edit the comaintainer list we have a UI for that already.
I am not too sure. The current implementation allows the maintainer to nominate a new maintainer and make himself a co-maintainer (by putting himself at the end of the list of co-maintainers and disowning the package).
My thought is that the disown button should not change the co-maintainers if the disowning user is the maintainer. A user that is the maintainer and co-maintainer of a package should have to disown twice to be removed as a maintainer and co-maintainer. I don't think this edge-case merits a second button. Regards, Mikael
On Mon, 05 Mar 2018 at 16:50:01, Mikael Blomstrand wrote:
[...] My thought is that the disown button should not change the co-maintainers if the disowning user is the maintainer. A user that is the maintainer and co-maintainer of a package should have to disown twice to be removed as a maintainer and co-maintainer.
That sounds like a great idea. Do we need any adjustments to your patch to get this feature, Mark?
participants (4)
-
Eli Schwartz
-
Lukas Fleischer
-
Mark Weiman
-
Mikael Blomstrand