Skip to content

fix(APIController): Allow self-test depending on token and not user-agent - #2175

Merged
nickvergessen merged 1 commit into
masterfrom
fix/apicontroller/self-test-token
Jan 31, 2025
Merged

fix(APIController): Allow self-test depending on token and not user-agent#2175
nickvergessen merged 1 commit into
masterfrom
fix/apicontroller/self-test-token

Conversation

@provokateurin

Copy link
Copy Markdown
Member

I would also like to use this endpoint without having a matching user-agent.
The token is a better method to verify the client can even handle push notifications, because in order to have a valid token it already needs to be registered for push notifications.

@provokateurin

Copy link
Copy Markdown
MemberAuthor

/backport to stable31

@github-actions

Copy link
Copy Markdown
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 8909 was 8700 (+2.4%)
Please check your code again. If you added a new test this can be expected and the base value in tests/Integration/base-query-count.txt can be increased.

@provokateurin
provokateurinforce-pushed the fix/apicontroller/self-test-token branch from 7e857a4 to 1634fdcCompareJanuary 30, 2025 16:27
@github-actions

Copy link
Copy Markdown
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 8909 was 8700 (+2.4%)
Please check your code again. If you added a new test this can be expected and the base value in tests/Integration/base-query-count.txt can be increased.

Signed-off-by: provokateurin <kate@provokateurin.de>
@nickvergessen
nickvergessenforce-pushed the fix/apicontroller/self-test-token branch from 1634fdc to 4c80910CompareJanuary 31, 2025 15:25
@github-actions

Copy link
Copy Markdown
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 8909 was 8700 (+2.4%)
Please check your code again. If you added a new test this can be expected and the base value in tests/Integration/base-query-count.txt can be increased.

@nickvergessen

Copy link
Copy Markdown
Member

The token is a better method to verify the client can even handle push notifications

The check was incomplete … It would have needed to extend to check if it's actually a permanent token and from there we should then have checked notifications_pushhash in addition and I don't think that's worth it for now, while we can't later on limit to which devices we want to push. So for now we can remove it.
I would bring it back and limit it to push the the requested device in the future.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@provokateurin@nickvergessen@skjnldsv