Closing a ticket

How to represent in ForgeFed the activity of closing a ticket? When you open a ticket, you use Create or Offer where the object is a Ticket. But how to change the ticket status, to mark is as resolved?

Currently, there’s the isResolved boolean property. Maybe we’ll also have a resolvedBy property specifying who resolved the ticket.

The most generic and standard way to update an object is the Update activity. The problem is, in S2S an Update contains the full object, not just the fields you changed! So it becomes impossible to reliably determine exactly what you changed. My opinion: Update should be used only for general updates where the best description that software can display to the user is “Alice updated object X” without any specific description of what changed. For specific changes, use specific activities that make it clear what the action is, what was meant to be changed.

PROPOSAL: Have a Resolve activity that resolves a Ticket. To reopen a ticket, use Undo, much like when unfollowing a user.

Thoughts? It’s very simple, so I think I’ll go ahead and implement this for now. Will change my code of course if we pick some different representation.

(Issue #98)

tl;dr skip to the last paragraph

After discussion on IRC and more thinking, we probably should decide between using Update for all objects edits, and using specific activities for specific changes.

Pros of using Update:

  • No need to add custom activities for the various changes, just use Update and the recipient figures out what changed
  • Perhaps less dev work, just implement a generic Update handler, it doesn’t even have to understand the type of the object being updated (but this is questionable because need to implement diffing for viewing and for determining exact changes when it matters for authorizing the change… see below)

Pros of using specific activities:

  • When observing an activity, it’s trivial to tell exactly what it changed; with Update it would be impossible to tell what the activity did
  • When observing the history of an object, telling what each Update did may be possible by diffing the Updates, but that works only if there’s no possibility for other activities to edit fields, otherwise the diffing would be confused. For example if some activity has a side effect that edits some field, the software observing the history would have to understand this activity and be able to simulate the side effect. Is there a good reason to be confident there’s a fixed set of activities that edit fields, and the edits can always be simulated?
  • Many activities make edits to objects (e.g. Follow edits a followers collection), and Update is only one of them. How do we decide what uses Update and what doesn’t? Suppose we decide that Update is for changes that don’t have any special side effects. Now merging an MR or applying a patch can’t use Update, because there’s a side effect. So whenever editing fields is a side effect of an activity that does something else, like applying a patch to a repo, any software viewing a history collection where it appears would have to understand it and be able to simulate the fields edits, otherwise the diffing of activities will get confused.

Clarification: We don’t need a custom activity for every single editable field, it just needs to be expressive enough for the kind of viewing forges offer, and the different distinguishable actions that have their own authorization rules. A simple rule: If an object history page displays some event in specific words (and not “user X updated object Y”) or there’s a dedicated button in UI to trigger this event, then we probably want to give is a dedicated activity type. There are just few of those per object type, so we it would be a small manageable number of custom activities to add. For example, a ticket’s data that isn’t in sub-objects is mostly just its textual content, perhaps who’s assigned to it, whether it’s resolved, the due date, maybe the tags if they aren’t in their own Collection… there are just few kinds of edits on these fields.

Obviously using Update means we don’t add custom vocabulary for kinds of changes, but I’m worried we’d end up making ourselves really struggle with that silly diffing of activities to determine what they did. The exact change is already known in advance, there’s specific intention to edit specific field(s), but instead of preserving that info, we’d be working with full objects and doing complicated processing to figure out that initial intention. There are fields related to each other, e.g. isResolved, resolvedBy and resolved all refer to the same event, of a ticket being resolved, so the software would be looking at them carefully, trying to figure out which of those 3 got changed and why and what it means and does resolvedBy match the actor of the Update, otherwise that’s quite confusing, etc. etc. :stuck_out_tongue:

Activity diffing may be required for determining authorization e.g. maybe some user is allowed to close/reopen a ticket, but not allowed to change its due date.

If we go for specific activities, it does mean we add custom activity types, but the implementation of those activities is extremely simple, no diffing is required, generating UI for viewing them would be trivial too. Forges already have pages and endpoints for specific actions like a “Close this ticket” button on a ticket page, right? They already handle changes in this way, and not via generic updates. So they’d simply have federation-supporting variants of those handlers, the “close this ticket” handler would support both a button click from a local user, and a remote e.g. Resolve activity coming from a remote user.

Can I please hear some good arguments in favor of using just Update for all the object changes? :stuck_out_tongue: otherwise, it seems so much simpler to add specific activities that do specific clear obvious edits, making authorization and display trivial to implement. We could still use Update, keeping it for the case where the display of “user X updated object Y” is enough, and perhaps specifying specific fields intended to be changed in that way (e.g. a ticket’s title and description). Then Update becomes trivial to handle too.

Note that most AP implementations use just S2S so they probably don’t care about viewing since they have their own custom C2S API for that, but C2S/viewing is in the spec, and they’re picking activities just for their own use while we’re writing a spec so we’re thinking bigger than covering the needs of one specific implementation.

Sorry for the late reply, I took some time to think about this. So, basically:

  • based on the AS spec, I don’t think Update is descriptive enough for us since it doesn’t define any way to describe what has changed. It’s basically a way to ping another actor to let them know that an object has changed, and that’s it. Theoretically you could detect what’s changed by comparing the Update activity with your local copy, but it doesn’t sound like a very robust option
  • we could introduce an activity called PartialUpdate or Edit that only sends the properties that have changed. My issue with this is that this feels like a “core” change that should happen on AS/AP rather than on forgefed
  • I’m OK with using a Resolve activity but I wonder how many more we’ll need to define. For example, what about changes to a ticket’s title or content? For these changes we could just use Update knowing that the information of what has changes (the title or the content) is lost, and the UI will simply show “the ticket has been edited X minutes ago”
  • w.r.t. Resolve, I would replace “isResolved” with a “status” property because a ticket (or a MR) can be in several states, for example FIXED, WONTFIX, CLOSED, OPEN. Unless maybe we can have both “isResolved” and “status”

@zplus, thanks for the comment!

  • For the partial update idea, I think there’s been some discussion about a possible Patch activity on SocialHub, it’s not a perfect solution but if it gets into the Fediverse we can consider to use it
  • How many more activities we’ll need: I hope not too many! We’ll see what to do about ticket title/content edits, it’s a different milestone so I’ll open a separate thread about it when it’s time :slight_smile: maybe Update will be good for this, maybe not, I haven’t looked closely yet
  • The “isResolved” property: I agree we want to have a ticket status! The reason there isn’t one yet, is that there’s no consensus yet on which values that “status” property would have, or whether it would be extensible or not. But it will probably need to be extensible, which means there’s no obvious way for implementations to tell whether a ticket is resolved or not since they may see a “status” value they don’t understand. So my thought is: Have a boolean ‘isResolved’ property, and in addition to that, have a property (or several properties) that describe the ticket status in more detail. The status of the work on it, or the reason is was closed, etc. etc. That way implementations always know whether a ticket is open or closed, even when the ticket status is something custom.

Sounds good?

@fr33domlover So, I’ve put more thought into this. Basically, I’ve always tried to be very conservative with any new properties and reuse AS vocabulary as much as possible, but given that AS is designed to match the typical usage of a “social network” (such as the activity of a microblogging), maybe we (I) should not be too shy about adding new properties because the meaning is different anyway. The biggest drawback I think, is that Activities are expected to be served in compact form and with RDF terms only (no short URIs like “ex:foobar”), probably with the idea of keeping JSON parsing as simple as possible. If somebody else decides to extend forgefed further, it might become increasingly difficult to avoid using prefixes in the JSONLD documents.
But anyway… for me it’s OK to keep both properties “isResolved” and “status” or “hasStatus”.

@zplus, interesting thoughts :slight_smile: the vocabulary we need for complex object is definitely more specific than the vocabulary that serves simple non-editable short plain text messages. I agree the expectation to use terms without a prefix is a problem. But it’s not specific to ForgeFed. To be honest I really really dislike JSON-LD and I wish we could just work with RDF/XML or any other sane representation and parse/convert it into whatever is easy to work with in whatever programming language we’re using. But JSON-LD was picked for AS2 and AP and now we’re stuck with it. Perhaps we can add a note to the spec, for plain-JSON implementations to accept property names both with and without prefix, and perhaps the fill URL too? e.g. in addition to recognizing isResolved, also recognize forge:isResolved and perhaps also https://forgefed.peers.community/ns#isResolved. Maybe that helps make the problem a bit smaller? Although in the bigger picture, it’s a problem with using JSON-LD and there isn’t much we can do within ForgeFed except maybe try to push for adding support for other RDF serializations to ActivityPub? Or remove JSON-LD from this universe lol

(One might ask, why not just implement JSON-LD wherever RDF/XML is implemented? Answer: Last time I checked, JSON-LD is a very, very complicated format with weird features useful only to JS devs insisting not to use RDF processing, and a huge pain for everyone else. The algorithms in the JSON-LD spec are very very hard to understand how they work and they’re basically a natural language description of their JS implementation, making it very difficult to implement in any way other than strictly following them line-by-line which may result with very slow/inefficient code, or even be very cumbersome and unreadable when implementing in programming paradigms other than the ones of JS… also, the algorithms being unreadable and originating in such a weak-dev-time-safety language as JS makes it very hard to debug the algorithms and find mistakes in them… and the way they’re written leaves tons of unhandled cases that just happen to mostly work out in the end but there’s no way to verify that from the algo itself, and that’s scary and results in bugs because people test the algos by using the JS implementation instead of by writing them in a mathematical style that covers all the cases in a clear way) taking a deep breath idk, no offense but is JSON-LD garbage or am I just being overly frustrated about it?)

Maybe you are a bit too frustrated about it :slight_smile: I don’t think jsonld is bad but, yes the same content can assume many forms so when you receive an activity or fetch an object you don’t know which form it is. AP has solved this by requiring the compact form and a single root node, but I think it doesn’t say anything about prefixes other than “Implementations which treat ActivityStreams objects as simply JSON rather than converting an incoming activity over to a local context using JSON-LD tooling should be aware of this and should be prepared to accept all three representations.”. And prefixes are arbitrary too btw, somebody could use “foobar: https://forgefed.peers.community/ns#” so the only reliable way would be to expand the URIs and check the full string. In practice though, this will only be a problem when parsing 3rd party objects that have been combined with other dictionaries, if those other dictionaries contain overlapping properties. The quick way around this is to expand the jsonld document first, and then compact it again with the forgefed context. In that way all the forgefed terms are compacted without prefix, and all other properties are left with the full URI. It’s not really an issue for us now, because we are not going to overlap with the AS terms, but I guess it’s an issue with pure json parsers if they need to work with any implementation that uses custom dictionaries, and they don’t know how to expand/compact jsonld.

1 Like

semantically, (in terms of the conventional CRUD operations)
‘update’ is the appropriate operation; but it is not creating,
reading, no destroying the record

IIRC, this was discussed during the mailing list discussions -
rather than specific boolean flags for each bit of relevant
state, there should be one ‘state’ field, corresponding to the
list of ticket “tags” or “labels” in the database - the usage
is common enough, that most of these are fairly conventional,
such as ‘open’ (the initial state, no tags, empty ‘state’ list),
‘closed’ could be one optional explicit value, along with ‘bug’,
not-a-bug’, ‘on-hold’, ‘needs-help’, and so on

when seen that way, this is more clearly an update operation -
that generic approach, also makes custom values possible, which
could be relevant to some forges, and safely ignored by others

regarding “who did this?”: isnt that obvious? - it is a design
goal that the identity of the sender of all messages will be
verifiable; and is especially important for messages that request
to modify the database

none of that suggest that there must be a dedicated field for
that one aspect of ticket state - there could simply be a few
special reserved values specified for the status field: with
‘closed’ and ‘locked’ being the most obvious - i suggested some
others that are not universal, but are common examples of “tags”;
which usually are arbitrary strings

the meanings of ‘closed’ and ‘locked’ are not arbitrary nor
ambiguous - they are universals, and will have a precise
interpretation on any issue tracking system - there is no risk of
collision in those identifiers or their semantics

the rationale that requires ‘is-closed’ to be a distinct field,
would also require ‘is-locked’ to be another distinct field; but
those concept are both better reasoned about, as equal aspects of
the general concept of “ticket state” - that is why i suggest
that all aspects of ticket state be delivered as a single CSV
‘status’ field; which may carry any number or arbitrary values,
reserving only a limited few special reserved keywords, such as
‘closed’ and ‘locked’ - that has better semantics, and avoids
bloating the API

there is no problem with allowing values to be arbitrary -
un-recognized values can simply be ignored - that is probably
true for all of the forge-fed data; because it it generally true
for any federated system - peers can never be trusted to be
sane; so all data must be validated

disallowing arbitrary values has exactly the same result as
allowing them, when the forge ignores the unrecognized value
(which it must be prepared to do anyways); so there absolutely no
reason to disallow it - indeed, it is not possible to disallow
it - that is especially true for the ticket status tags; because
those are known to be arbitrary strings on most forges

for these status indicators, it is not difficult for the forge to
know how to handle them - even on an issue tracker that has no
concept of “read-only”, upon seeing the status: ‘locked’ it
would simply be seen as an arbitrary tag, just the same as:
‘my-custom-tag’ - the forge could simply create a new generic tag
for it (if it has such a feature), in the same way as the local
user would do with a mouse, or simply ignore it

perhaps that was a long-winded argument; but simplicity and
concision is always best - the place for verbosity is here,
during the design phase, in order to avoid unnecessary verbosity
in the API