Skip to content

Commit

Permalink
[wip] Add documentation regarding the notification API
Browse files Browse the repository at this point in the history
  • Loading branch information
tarsius committed Oct 8, 2023
1 parent 84e7040 commit acc0206
Show file tree
Hide file tree
Showing 3 changed files with 337 additions and 15 deletions.
169 changes: 162 additions & 7 deletions docs/forge.org
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,21 @@ available from the ~magit-pull~ transient (on ~F~).
- Key: f N (forge-pull-notifications) ::
- Key: N f n ::

This command uses a forge's API to fetch all notifications from that
forge including, but not limited to, the notifications for the current
repository.
This command uses Github's API to fetch all notifications from
that forge, including, but not limited to, the notifications for
the current repository.

Other forges are currently not supported.

Fetching all notifications fetches associated topics even if you
have not started fetching *all* topics for the respective repositories
(using ~forge-pull~), but it does not cause the topics to be listed in
the status buffer of such "uninitialized" repositories.
Unfortunately Github's notification API is so unbelievably bad that
Forge cannot fully work around its defects. As a consequence users
have to know about these defects and have to actively take part in
their mitigation. See [[*Dealing with Github's abysmal notification API]].

With a prefix argument fetch all notifications, not just those
that Github considers as updated since the last pull. This is
useful after you have marked a notification as unread, using the
web interface, because Github doesn't consider that an update.

Note how pulling data from a forge's API works the same way as pulling
Git data does; you do it explicitly when you want to see the work done
Expand Down Expand Up @@ -997,6 +1004,154 @@ The following commands are available in buffers used to edit posts:
This command toggles whether the pull-request being created is a
draft.

** Dealing with Github's abysmal notification API

There is only ever one notification about any given entity (such as an
issue or commit). Even if someone comments on an ancient issue, which
was resolved years ago, the notification that you receive about that
event, is technically the same notification (object), as when you were
first informed about the issues creation. In other words, a
notification is just some additional set of metadata about some
entity, that tracks whether and how, you have interacted with it since
someone else interacted with it.

It doesn't have to be implemented like this, but it is a reasonable
approach and the one that Github chose. Unfortunately it's all
downhill from here.

*** Missing Booleans in API Response
:PROPERTIES:
:UNNUMBERED: notoc
:END:

Some very important pieces of information in a notification are
whether you have viewed the referenced entity, since the last time
someone else has acted on it; whether you have marked the notification
as done or not; and whether you have marked it as needing some special
attention. The two important booleans are "read/unread" and
"done/pending"; the third, "marked/unmarked", is comparatively
insignificant.

(Note that, different booleans are needed to answer the questions "Am
I done with this notification?" and "Has this issue been closed". You
might, for example, close a feature request because you do not intend
to implement it, but if someone comments on the issue, after you have
done that, you have to read that (changing the "read/unread" boolean).
You might decide to immediately reopen the issue or respond by saying
that you stick to your decision. However, you might also decide that
you first want to think about it some more. I.e., the issue stays
"closed" but the respective notification stays "pending".)

Github's web interface provides all three of these boolean states, but
th notification API only provides the first, "read/unread". Yet, we
simply cannot do without the "done/pending" boolean, and since Github
does not volunteer that piece of information, we have to guess the
value, as best as we can.

(Whether you have "marked" a notification is less important, so Forge
for now does not try to fake it. One reason for that is that I have
not yet decided whether I want to use a boolean or allow for multiple
distinct marks. One might argue that we might as well do that latter,
since syncing with Github isn't possible anyway. Then again, maybe it
is enough that topics themselves can be have multiple private marks
and/or public labels.)

Forge sets the "done/pending" boolean based on the value of the
"read/unread" boolean, which obviously is completely wrong, but also
the best we can do. It works like this:

1. If the API response says that the notification is "unread" then
Forge always considers the notification "pending" as well.

That matches the behavior of the web interface: there too, every
notification that is "unread" is also always "pending". It's also
the obviously correct thing to handle this.

2. Otherwise, if the API response says that the notification has
already been "read", then Forge's behavior depends on whether the
notification about the entity in question has been fetched before.

If the same notification has been fetched before, then the value
of the "done/pending" boolean is always left untouched.

If the notification was just fetched for the first time, then Forge
marks the notification as "done".

(If a notification has already been fetched before Forge was
updated to a version that behaves as described here, then it as
considered previously unfetched, the first time it is fetched again
after Forge was updated.)

Unless you never look at notifications using the web interface, or you
always mark every notification as "unread" right after reading it,
using the web interface, you will often end up with notifications that
are falsely considerd "done" by Forge.

TODO Document the other approach instead (or also?).

When you notice that this has happened, list all (locally known as)
"unread" notifications, using ~n l n~. (At least this is the default
behavior of ~forge-list-notifications~, you can configure it to list a
different set of notifications). Also visit
https://github.com/notifications in browser. The same notifications
should be listed in both places.

In the web interface mark all notifications that are missing from
the local list as unread. Then invoke this command with a prefix
argument to force the last three hundred or so notifications to be
fetched again. After doing that these notifications are "pending"
and "unread" both in the web interface and locally.

*** Missing State Mutations
:PROPERTIES:
:UNNUMBERED: notoc
:END:

Synchronizing changes in the other direction, from Forge to Github,
also only works partially.

What works is that when you visit the entity corresponding to a
previously "unread" notification in Forge, then it is marked as
"read", not only in Forge itself, but also on Github.

However, if you locally mark a notification as "unread", "done" or
"pending", then that does not update the corresponding boolean on
Github. Once more the reason is that Github's API is abysmal when
it comes to notifications. It simply does not support making these
state changes.

*** Missing GraphQL API
:PROPERTIES:
:UNNUMBERED: notoc
:END:

Github's GraphQL API does not support notifications, which is ironic
because if it did, then that could be used as an excellent example of
the benefits of GraphQL (of course, assuming all the defects of the
respective REST API are addressed).

Forge deals with this by first fetching the list of notifications
using the REST API. Then it constructs a GraphQL query that fetched
the referenced entities (topics, commits etc.).

This is merely an inconvenience, which makes the whole process slower,
but would not be worth mentioning here, if it did not contribute to
the next defect.

*** Missing Entity Relation
:PROPERTIES:
:UNNUMBERED: notoc
:END:

You thought it could not get any worse? Well, hold on to your hat.
When it comes to notifications about discussions, then information
that is even more fundamental than the state booleans, discussed
above, is missing. For a discussion notification it is not even
possible to unambiguously determine which discussion it refers to.

Forge does not support discussions at all yet, but I am working on it.
Guess what the primary holdup is.

** Miscellaneous

- Key: N c f (forge-fork) ::
Expand Down
170 changes: 162 additions & 8 deletions docs/forge.texi
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Usage
* Pulling::
* Branching::
* Working with Topics::
* Dealing with Github's abysmal notification API::
* Miscellaneous::
Working with Topics
Expand Down Expand Up @@ -699,6 +700,7 @@ binding you can substitute @code{'} for that.
* Pulling::
* Branching::
* Working with Topics::
* Dealing with Github's abysmal notification API::
* Miscellaneous::
@end menu

Expand Down Expand Up @@ -731,14 +733,21 @@ and issues. See @ref{Initial Pull}.
@kindex f N
@kindex N f n
@findex forge-pull-notifications
This command uses a forge's API to fetch all notifications from that
forge including, but not limited to, the notifications for the current
repository.

Fetching all notifications fetches associated topics even if you
have not started fetching @strong{all} topics for the respective repositories
(using @code{forge-pull}), but it does not cause the topics to be listed in
the status buffer of such "uninitialized" repositories.
This command uses Github's API to fetch all notifications from
that forge, including, but not limited to, the notifications for
the current repository.

Other forges are currently not supported.

Unfortunately Github's notification API is so unbelievably bad that
Forge cannot fully work around its defects. As a consequence users
have to know about these defects and have to actively take part in
their mitigation. See @ref{Dealing with Github's abysmal notification API}.

With a prefix argument fetch all notifications, not just those
that Github considers as updated since the last pull. This is
useful after you have marked a notification as unread, using the
web interface, because Github doesn't consider that an update.
@end table

Note how pulling data from a forge's API works the same way as pulling
Expand Down Expand Up @@ -1322,6 +1331,151 @@ This command toggles whether the pull-request being created is a
draft.
@end table

@node Dealing with Github's abysmal notification API
@section Dealing with Github's abysmal notification API

There is only ever one notification about any given entity (such as an
issue or commit). Even if someone comments on an ancient issue, which
was resolved years ago, the notification that you receive about that
event, is technically the same notification (object), as when you were
first informed about the issues creation. In other words, a
notification is just some additional set of metadata about some
entity, that tracks whether and how, you have interacted with it since
someone else interacted with it.

It doesn't have to be implemented like this, but it is a reasonable
approach and the one that Github chose. Unfortunately it's all
downhill from here.

@anchor{Missing Booleans in API Response}
@subheading Missing Booleans in API Response

Some very important pieces of information in a notification are
whether you have viewed the referenced entity, since the last time
someone else has acted on it; whether you have marked the notification
as done or not; and whether you have marked it as needing some special
attention. The two important booleans are "read/unread" and
"done/pending"; the third, "marked/unmarked", is comparatively
insignificant.

(Note that, different booleans are needed to answer the questions "Am
I done with this notification?" and "Has this issue been closed". You
might, for example, close a feature request because you do not intend
to implement it, but if someone comments on the issue, after you have
done that, you have to read that (changing the "read/unread" boolean).
You might decide to immediately reopen the issue or respond by saying
that you stick to your decision. However, you might also decide that
you first want to think about it some more. I.e., the issue stays
"closed" but the respective notification stays "pending".)

Github's web interface provides all three of these boolean states, but
th notification API only provides the first, "read/unread". Yet, we
simply cannot do without the "done/pending" boolean, and since Github
does not volunteer that piece of information, we have to guess the
value, as best as we can.

(Whether you have "marked" a notification is less important, so Forge
for now does not try to fake it. One reason for that is that I have
not yet decided whether I want to use a boolean or allow for multiple
distinct marks. One might argue that we might as well do that latter,
since syncing with Github isn't possible anyway. Then again, maybe it
is enough that topics themselves can be have multiple private marks
and/or public labels.)

Forge sets the "done/pending" boolean based on the value of the
"read/unread" boolean, which obviously is completely wrong, but also
the best we can do. It works like this:

@enumerate
@item
If the API response says that the notification is "unread" then
Forge always considers the notification "pending" as well.

That matches the behavior of the web interface: there too, every
notification that is "unread" is also always "pending". It's also
the obviously correct thing to handle this.

@item
Otherwise, if the API response says that the notification has
already been "read", then Forge's behavior depends on whether the
notification about the entity in question has been fetched before.

If the same notification has been fetched before, then the value
of the "done/pending" boolean is always left untouched.

If the notification was just fetched for the first time, then Forge
marks the notification as "done".

(If a notification has already been fetched before Forge was
updated to a version that behaves as described here, then it as
considered previously unfetched, the first time it is fetched again
after Forge was updated.)
@end enumerate

Unless you never look at notifications using the web interface, or you
always mark every notification as "unread" right after reading it,
using the web interface, you will often end up with notifications that
are falsely considerd "done" by Forge.

TODO Document the other approach instead (or also?).

When you notice that this has happened, list all (locally known as)
"unread" notifications, using @code{n l n}. (At least this is the default
behavior of @code{forge-list-notifications}, you can configure it to list a
different set of notifications). Also visit
@uref{https://github.com/notifications} in browser. The same notifications
should be listed in both places.

In the web interface mark all notifications that are missing from
the local list as unread. Then invoke this command with a prefix
argument to force the last three hundred or so notifications to be
fetched again. After doing that these notifications are "pending"
and "unread" both in the web interface and locally.

@anchor{Missing State Mutations}
@subheading Missing State Mutations

Synchronizing changes in the other direction, from Forge to Github,
also only works partially.

What works is that when you visit the entity corresponding to a
previously "unread" notification in Forge, then it is marked as
"read", not only in Forge itself, but also on Github.

However, if you locally mark a notification as "unread", "done" or
"pending", then that does not update the corresponding boolean on
Github. Once more the reason is that Github's API is abysmal when
it comes to notifications. It simply does not support making these
state changes.

@anchor{Missing GraphQL API}
@subheading Missing GraphQL API

Github's GraphQL API does not support notifications, which is ironic
because if it did, then that could be used as an excellent example of
the benefits of GraphQL (of course, assuming all the defects of the
respective REST API are addressed).

Forge deals with this by first fetching the list of notifications
using the REST API@. Then it constructs a GraphQL query that fetched
the referenced entities (topics, commits etc.).

This is merely an inconvenience, which makes the whole process slower,
but would not be worth mentioning here, if it did not contribute to
the next defect.

@anchor{Missing Entity Relation}
@subheading Missing Entity Relation

You thought it could not get any worse? Well, hold on to your hat.
When it comes to notifications about discussions, then information
that is even more fundamental than the state booleans, discussed
above, is missing. For a discussion notification it is not even
possible to unambiguously determine which discussion it refers to.

Forge does not support discussions at all yet, but I am working on it.
Guess what the primary holdup is.

@node Miscellaneous
@section Miscellaneous

Expand Down
Loading

0 comments on commit acc0206

Please sign in to comment.