Details

Reviewers

avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15137: Not able to edit a "Calendar Imported .ics URI"

Commits

rP02a4f8b0c8f1: Fix editing a Calendar import ICS URI

Summary

Before this change it was only possible to create a Calendar import ICS URI.
After this change it's possible to also edit already-existing elements.

This change fixes this specific exception when visiting similar pages:

/calendar/import/edit/5/

Argument 2 passed to PhabricatorCalendarImport::initializeNewCalendarImport()
must be an instance of PhabricatorCalendarImportEngine, null given, called in
phorge/src/applications/calendar/editor/PhabricatorCalendarImportEditEngine.php
on line 45

Before this change it was only theorically possible to edit the name, policies, etc.
but not the URI.

This change also introduces the ability to edit the specific ICS URI,
in order to change legitimate parts of the URI, like the secret token.

Closes T15137

Test Plan

I tested in my own installation with success lints.

To test, visit the Calendar, create an import ICS URI, and then try to edit it again.
It will work only after this change.

I was not able to conclude the "arc diff" since it tries to connect
to an unexisting database owned by root.

Diff Detail

Repository

rP Phorge

Branch

phorge-T15137

Lint

Lint Passed

Unit

Tests Skipped

Build Status

Buildable 101
Build 101: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Nov 28 2022, 20:50

Owners added a reviewer: O1: Blessed Committers.Nov 28 2022, 20:50

Herald added subscribers: Matthew, tobiaswiese, speck. · View Herald TranscriptNov 28 2022, 20:50

valerio.bozzolan requested review of this revision.Nov 28 2022, 20:50

Harbormaster completed remote builds in B95: Diff 291.Nov 28 2022, 20:50

fix master ref

Harbormaster completed remote builds in B96: Diff 292.Nov 28 2022, 21:00

fix line 80 chars long

Harbormaster completed remote builds in B97: Diff 293.Nov 28 2022, 21:01

fix lint again

Harbormaster completed remote builds in B98: Diff 294.Nov 28 2022, 21:02

valerio.bozzolan retitled this revision from Fix NULL pointer exception from Calendar's homepage to Fix editing a Calendar import ICS URI.Nov 28 2022, 21:03

valerio.bozzolan edited the summary of this revision. (Show Details)

valerio.bozzolan added a task: T15137: Not able to edit a "Calendar Imported .ics URI".

Cigaryno subscribed.Dec 16 2022, 10:36

valerio.bozzolan edited the test plan for this revision. (Show Details)Dec 16 2022, 22:21

sorry if I added this newline - now the changeset is even shorter!

Harbormaster completed remote builds in B101: Diff 300.Dec 16 2022, 22:27

I was not able to conclude the "arc diff" since it tries to connect to an unexisting database owned by root.

I think that if you specify user/password in ./conf/local/local.json, the tests would use that.
In older times, mysql would just error out on missing auth, but at some point Ubuntu (and others?) integrated the mysql auth to the system's accounts, and made it more confusing to connect to it.

This revision is now accepted and ready to land.Dec 17 2022, 08:06

run tests

Harbormaster completed remote builds in B102: Diff 301.Dec 17 2022, 10:40

Thanks!

It seems I'm not able to land since I'm not a Blessed Committers sigh.

I have to report that all unit tests are OK but resources/celerity/map.php seems not updated (after rebranding I think). I will not update it, since it's unrelated from these Calendar changes.

So it's ready to land (if somebody-else can).

In D25061#2018, @valerio.bozzolan wrote:

Thanks!

It seems I'm not able to land since I'm not a Blessed Committers sigh.

Fixed that :)

I have to report that all unit tests are OK but resources/celerity/map.php seems not updated (after rebranding I think). I will not update it, since it's unrelated from these Calendar changes.

So it's ready to land (if somebody-else can).

This revision was landed with ongoing or failed builds.Dec 17 2022, 11:17

Closed by commit rP02a4f8b0c8f1: Fix editing a Calendar import ICS URI. · Explain Why

This revision was automatically updated to reflect the committed changes.

valerio.bozzolan added a commit: rP02a4f8b0c8f1: Fix editing a Calendar import ICS URI.

20after4 awarded a token.Dec 20 2022, 02:11

valerio.bozzolan mentioned this in 2023 Week 2.Mar 13 2023, 18:26

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

aklapper added a reverting change: D25411: Fix PHP 8.1 "strlen(null)" exceptions importing ICS file into calendar.Aug 18 2023, 16:40

valerio.bozzolan mentioned this in Wikimania Hackathon 2023 Singapore.Aug 19 2023, 04:47

aklapper added a reverting change: rP9b105c8e9e54: Fix PHP 8.1 "strlen(null)" exceptions importing ICS file into calendar.Aug 19 2023, 08:09

valerio.bozzolan mentioned this in Wikimania Hackathon 2023 Singapore.Aug 28 2023, 16:39