Fix handling of logout event

[zeripath]

It appears that there is a slight bug in the handling of the data of logout event -
the javascript should be testing the data field of the data field for the logout
instruction.

Signed-off-by: Andrew Thornton [email protected]

[silverwind]

Don't you need to JSON.parse(event.data) like it's done in receiveUpdateCount? Also that method uses uppercase property access like .Count so maybe it's .Data actually?

[zeripath]

Don't you need to JSON.parse(event.data) like it's done in receiveUpdateCount?

Don't appear to need it. Have you found that you do need it?

---

Looking again at this - the data is not JSON data so no it does not need to be parsed no.

---

Also that method uses uppercase property access like .Count so maybe it's .Data actually?

Which method?

[silverwind]

Just a few lines above event.data is passed into receiveUpdateCount:

gitea/web_src/js/features/notification.js

Line 65 in 4a68ed4

receiveUpdateCount(event.data);

which then does JSON.parse on the original event.data.data (argument to the function is deceivingly mislabeled as event, it should be data):

gitea/web_src/js/features/notification.js

Line 23 in 4a68ed4

const data = JSON.parse(event.data);

And finally it accesses title-cased .Count on the parsed object:

gitea/web_src/js/features/notification.js

Line 32 in 4a68ed4

notificationCount.textContent = `${data.Count}`;

I think this requires more refactoring to be clear. I would JSON.parse immediately in the event handler and then pass around the parsed object only.

[zeripath]

the data from a logout event is simply text. So this is correct as it stands.

[silverwind]

The only way this can work is that in the event notification-count, data is a JSON-encoded string while for logout it is just the here string (and invalid JSON). I think for clarity we should just always send JSON-parseable data.

[zeripath]

No you've got it wrong. Why don't you test it.

[silverwind]

That's something I've meaning to ask. How does one generate fake eventsource events on the server? Is there some convenient way to do so?

[zeripath]

there's no built in way of doing this. All of the events are created in routers/events.go:

gitea/routers/events/events.go

Lines 35 to 40 in 426ebbf

	event := (&eventsource.Event{
	Name: "close",
	Data: "unauthorized",
	})
	_, _ = event.WriteTo(ctx)
	ctx.Resp.Flush()

is the simplest example.

In this case the important event is:

gitea/routers/events/events.go

Lines 130 to 134 in 426ebbf

	_, _ = (&eventsource.Event{
	Name: "logout",
	Data: "here",
	}).WriteTo(ctx.Resp)
	ctx.Resp.Flush()

Which will simply be emitted as:

event: logout
data: here

The eventsource will then send a js object event:

{
  ...
  "data": {
    "type": "logout",
    "data": "here",
  }
}

which gets pushed to the workers as an "event":

{
   "type": "logout",
   "data": {
    "type": "logout",
    "data": "here",
   },
}

The double wrapping of the data is an unfortunate consequence of the way events are passed around js - this could be stopped by changing:

gitea/web_src/js/features/eventsource.sharedworker.js

Lines 50 to 54 in 426ebbf

	this.eventSource.addEventListener(eventType, (event) => {
	this.notifyClients({
	type: eventType,
	data: event.data
	});

L53 to event.data.data instead of event.data but then you lose the ID and Retry time should we wish to use those.

---

In the case of notification events the data is []byte of json data in the Event struct.

[zeripath]

I need to move the stopwatches query out of the event stream and instead consider a similar db query to how we do the notification stream but I've not had the inclination do that yet.

[silverwind]

I think I understand now. My assumption was that the inner data would always be a JSON string but that's just not the case (would still prefer if we would not mix types like that for different events).

Event{
  Name: "notification-count",
  Data: uidCount,
}

Event{ 
  Name: "logout", 
  Data: "here", 
}

# uidCount 
type UserIDCount struct {
	UserID int64
	Count  int64
}

I did not find where the JSON serialization happens but it must be somewhere.

Refactoring this data.data stuff would certainly be welcome for another PR.