Summary

Create PurpleAuthorizationRequest and use it for notifications.

Review Request #1613 — Created Aug. 21, 2022 and submitted Aug. 23, 2022, 3:14 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

~~1612~~

Reviewers

Groups

pidgin

People

Description

This replaces the old internal representation of authorization requests as well
as the UI's implementation of their own objects. Everything is now controlled
via PurpleAuthorizationRequest and the UI's display the notification for
interaction.

Testing Done

Verified the notification and actions work in both finch and pidgin.

Commits

Summary	ID
Create PurpleAuthorizationRequest and use it for notifications. This replaces the old internal representation of authorization requests as well as the UI's implementation of their own objects. Everything is now controlled via PurpleAuthorizationRequest and the UI's display the notification for interaction.	4d84dcc4316008256e8d8411b10c5a3508167a61

Issues

Description	From	Last Updated
s/the //	QuLogic	Aug. 23, 2022, 2:45 a.m.
Fits in one line?	QuLogic	Aug. 23, 2022, 2:45 a.m.
Not sure we need G_PARAM_CONSTRUCT for this and the remaining properties, as we don't use the constructed vfunc.	QuLogic	Aug. 23, 2022, 2:45 a.m.
So formerly, you could hit the X on the minidialog to ignore requests, and they'd just come back the next …	QuLogic	Aug. 23, 2022, 2:59 a.m.
Doesn't look like you ref'd authorization_request in here, so it should be marked as a transfer full argument (or should …	QuLogic	Aug. 23, 2022, 2:45 a.m.
destroy	QuLogic	Aug. 23, 2022, 2:45 a.m.
But message is not NULL?	QuLogic	Aug. 23, 2022, 2:45 a.m.
Align	QuLogic	Aug. 23, 2022, 2:45 a.m.

Change Summary:

handle deleting notifications in a sane way.

Testing Done:

		Verified the notification and actions work in both finch and pidgin.
-
-		There's currently an issue where deleting the notification will hide the notification, but the request still exists. I'm not sure which way we want to go with this, but figured we can start reviewing with out it.

Commits:

	Summary	ID
	Create PurpleAuthorizationRequest and use it for notifications. This replaces the old internal representation of authorization requests as well as the UI's implementation of their own objects. Everything is now controlled via PurpleAuthorizationRequest and the UI's display the notification for interaction.	593f2602df52774044166d7021a941e2ef14e8d0
	Create PurpleAuthorizationRequest and use it for notifications. This replaces the old internal representation of authorization requests as well as the UI's implementation of their own objects. Everything is now controlled via PurpleAuthorizationRequest and the UI's display the notification for interaction.	f9c5b1ef4e6cafa60f99af11a112b6f538a3c635

Diff:

Revision 2 (+3410 -1132)

Show changes

	ChangeLog.API
	finch/finchnotifications.c
	finch/gntaccount.c
	libpurple/account.h
	libpurple/account.c
	libpurple/accounts.h
	libpurple/meson.build
	libpurple/purpleauthorizationrequest.h
	14 more

libpurple/purpleauthorizationrequest.h (Diff revision 2)
Show all issues
```
s/the //
```
libpurple/purpleauthorizationrequest.c (Diff revision 2)
Show all issues
```
Fits in one line?
```

libpurple/purpleauthorizationrequest.c (Diff revision 2)

Show all issues

Not sure we need G_PARAM_CONSTRUCT for this and the remaining properties, as we don't use the constructed vfunc.

libpurple/purplenotification.c (Diff revision 2)

Show all issues

So formerly, you could hit the X on the minidialog to ignore requests, and they'd just come back the next time you'd login.

I'm not sure if we want to keep that behaviour or not. It's certainly going to be easier to look at notifications now in this dialog that you probably won't accidentally close instead of hitting left or right.

grim Aug. 23, 2022, 2:31 a.m.

Yeah I wasn't sure which way to go, but I figured if a user x'd it out they didn't want to see it anymore in this use case.

QuLogic Aug. 23, 2022, 2:47 a.m.

Just wondering if maybe people are wanting an ignore-for-now sort of effect, or maybe don't want to notify the other person they even reacted to their request (e.g., for spam or harassment avoidance purposes). But maybe some of that could be protocol-specific changes, and we can design that later.

grim Aug. 23, 2022, 2:49 a.m.

yeah or we could make a preference too? But the question is, what do we want to do here, remove or leave it?

QuLogic Aug. 23, 2022, 2:51 a.m.

Let's keep the signal, but drop the deny, just to keep things working the same as before.

grim Aug. 23, 2022, 2:52 a.m.
```
sounds good!
```

libpurple/purplenotification.c (Diff revision 2)

Show all issues

Doesn't look like you ref'd authorization_request in here, so it should be marked as a transfer full argument (or should be ref'd (and then unref'd in all callers)).

libpurple/tests/test_authorization_request.c (Diff revision 2)
Show all issues
```
destroy
```
libpurple/tests/test_authorization_request.c (Diff revision 2)
Show all issues
```
But message is not NULL?
```
pidgin/pidginnotificationauthorizationrequest.c (Diff revision 2)
Show all issues
```
Align
```