Uploading the patch that works on my computer now®

Thanks for this patch that unfortunately it's revealing even more weird legacy ghosts.

Mark as "some comments"

Should ideally use the same error message as in https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php;4862eada5cd05236b81487b261668f2a2d72fad7$382-387 so one string less to translate.

Covered, thanks for review :3

I need to sleep

transaction.search: handle unmanaged exception when objectType is not supported

Thaanks. Can reproduce in latest master. Premising that uhmmmm probably it seems we are not using the API correctly as the correct usage seems with objectType=TASK. Example to get my transactions:

add more edge cases in unit tests

Don't think this is needed for a standard install. The gerritbot colors could be implemented only on phabricator.wikimedia.org.

The colors indicate patch status (the bolded words in the comment) – blue for new, green for merged, red for abandoned.

To put it another way:
I think the described use-case is too narrow, and a naive expansion of the use-case isn't scalable.
The described use-case fails for at least one possible use-case ("some bots have something useful to say").

In D25987#26487, @valerio.bozzolan wrote:

having "User Bot" possibly everywhere? 🌈

Personally I'd decline T16052 (data duplication) and I do not yet see a need for potential new transaction types either (a comment is a comment is a comment no matter who/what made it).

Yes, this is where my first comment enters - I feel this is a rabbit-hole we shouldn't venture into, etc.
The current script appears to have 3 names, and referring to "legacy data" implies that there won't be any new names to add.

If you mean this kind of hardcoded CSS rules, yes, it's possible for local installs:

the legacy data can be handled by the already-existing hard-coded names...

Yessss, I agree and we explored a bit the creation of new transactions as good long-term direction, unfortunately it seems still necessary a bit of CSS help from the backend to cover the legacy data (e.g. 10 years old bot-generated comments, with traditional comments)

I feel this is a rabbit-hole we shouldn't venture into.
The slippery-slope argument will make us adding a custom class for each individual user, so css extensions can be used to hide/highlight comments from boss/intern/etc.
It's also probably not enough to remove the hard-coded requirement either - in some environments, one "bot" user is copying comments from another platform, and another is making statistical updates about a jira ticket, so you'd still need a better filter.

Uhm... can we sleep with the very possible possibility of having "User Bot" possibly everywhere? 🌈

removed nonsense comment (last change.zip)

always set a bool so we have an explicit value and don't just fallback to "false"

Don't bool with the bool otherwise it bools

fix damn 81 lines lint

add destroy unit test for A > B > C and milestone (I love computers)

The docs are surprisingly less jokey than I feared. Left a few minor change suggestions, as well as in the code comments.

Note: the same issue appears to also exist for images:

look mum, I'm learning English

improve comments (again) - thanks

@matmarex why are the gerritbot colors different in the "after" mockup?

avoid double unset(), just use or

In D25984#26354, @valerio.bozzolan wrote:

Should we phlog($e) in your opinion?

After a conversation with valerio, I probably had misunderstood the feature scope in my comment D25363#18825: When I tested if this also matches for a user who is not the user importing the event into the calendar.
As a note to myself, the commit message says:

WE DO NOT MATCH OTHER USERS BUT THE CALENDAR OWNER.

update inline doc, thanks aklapper

Seems to work locally.

Yeah, definately very useful. We have been able to integrate all our services with AWS SSO trhough SAML, except Phorge, so it would be great to keep all together.

I've tested this again in my production on gitpull.it (lol) and I cannot see whatsoever change in the resulting project report (and I'm sure that the page I'm testing is the one affected since I've introduced some logs in-code). So, approving 👍

I agree about the mystery in the test plan. Anyway, easily test-able by replacing $this->getMetadataValue('stub:create:phid') with a PHID lol. Thaaaanks all.