Summary

Add Purple.Account:protocol

Review Request #3643 — Created Nov. 5, 2024 and submitted Nov. 10, 2024, 6:56 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Blocks

3644

Reviewers

Groups

pidgin

People

Description

This is construct only property which allows us to do more complicated testing.
This also caches lookups based on Purple.Account:protocol-id.

Testing Done


Opened in a devenv and verified my accounts worked.
Started creating a new account and verified that changing the protocol didn't throw any warnings.
Modified an existing account by changing its protocol without issue.
Deprecated purple_account_get_protocol_name as there's no good reason for it.
Called in the turtles.

Commits

Summary	ID
Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	5eb57eb5df7d1930694e1ca09108b32bb645ab66

Issues

Description	From	Last Updated
Since no callers changed, I'm a bit uncertain why both protocol and protocol-id need to be settable? Can we not …	QuLogic	Nov. 10, 2024, 3:41 a.m.
Should this be g_assert_no_error like the other time?	QuLogic	Nov. 10, 2024, 6:44 a.m.

Change Summary:

Fixed the mistake where I updated get_id and not get_protocol_id. Also make get_id generate the id if one doesn't exist yet which allows us to make that property a non-construct property.

Commits:

	Summary	ID
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	401f4b0cdfa2421c910814f9131e40441a253ca9
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	eae5ce23d9c4fd0ee1fa1888d06a7d8421fa0bdd

Diff:

Revision 2 (+202 -70)

Show changes

	libpurple/purpleaccount.h
	libpurple/purpleaccount.c

The issue has been dropped. Show all issues

Since no callers changed, I'm a bit uncertain why both protocol and protocol-id need to be settable? Can we not cache protocol on first use from protocol-id or vice-versa, or are there other changes planned?

grim Nov. 5, 2024, 6:07 a.m.

protocol is construct only so we can set it from a unit test with g_object_new(PURPLE_TYPE_ACCOUNT, "protocol", prpl1, NULL);.
protocol-id can be written whenever which we need to be able to do to let users change the protocol in the account editor. However, we don't need a protocol in this case until purple_account_get_protocol is called and when that happens we look up the protocol in the ProtocolManager like we did before, but this time we cache it.
In the unit tests, we don't have the protocol manager unless we initialize all of libpurple which means our tests will be slower, it's possible other protocols might interfer, and so on. So to avoid all that, being able to create an account with an instance of a test protocol like in this review request makes things much easier and faster to run.

QuLogic Nov. 5, 2024, 6:14 a.m.

So there's no need for some corresponding test changes then, yet?

grim Nov. 5, 2024, 6:15 a.m.

We could add tests to check it now, but I made this explicitly to enable testing in /r/3644/

Change Summary:

Add unit tests for accounts including creating them with a protocol and a protocol-id.

Commits:

	Summary	ID
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	eae5ce23d9c4fd0ee1fa1888d06a7d8421fa0bdd
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	296d3a51007c02b2289d500da6fe8af452f5f1a0

Diff:

Revision 3 (+682 -70)

Show changes

	libpurple/purpleaccount.h
	libpurple/purpleaccount.c
	libpurple/tests/meson.build
	libpurple/tests/test_account.c

libpurple/tests/test_account.c (Diff revision 3)
The issue has been resolved. Show all issues
```
Should this be g_assert_no_error like the other time?
```

Change Summary:

fix unit test

Commits:

	Summary	ID
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	296d3a51007c02b2289d500da6fe8af452f5f1a0
	Add Purple.Account:protocol This is construct only property which allows us to do more complicated testing. This also caches lookups based on Purple.Account:protocol-id.	5eb57eb5df7d1930694e1ca09108b32bb645ab66

Diff:

Revision 4 (+682 -70)

Show changes

	libpurple/purpleaccount.h
	libpurple/purpleaccount.c
	libpurple/tests/meson.build
	libpurple/tests/test_account.c

Ship it!

```
Ship It!
```

Status:: Completed