Page MenuHomePhorge

Fix editing a Calendar import ICS URI
ClosedPublic

Authored by valerio.bozzolan on Nov 28 2022, 20:50.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 19:13
Unknown Object (File)
Tue, Mar 26, 17:16
Tokens
"Mountain of Wealth" token, awarded by 20after4.

Details

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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 edited the test plan for this revision. (Show Details)

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

avivey subscribed.

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

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).

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
This revision was automatically updated to reflect the committed changes.