Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.x] Replace jenssegers/agent and use latest mobiledetect/mobiledetectlib #1399

Merged
merged 20 commits into from
Nov 7, 2023

Conversation

crynobone
Copy link
Member

@crynobone crynobone commented Nov 6, 2023

  • Port Jenssegers\Agent\Agent to Laravel\Jetstream\Agent
    • Without isRobot() detection

crynobone and others added 7 commits November 3, 2023 18:48
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
@crynobone crynobone marked this pull request as draft November 6, 2023 05:00
crynobone and others added 5 commits November 6, 2023 13:05
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
src/Agent.php Outdated
if (
$this->getUserAgent() === static::$cloudFrontUA
&& $this->getHttpHeader('HTTP_CLOUDFRONT_IS_DESKTOP_VIEWER') === 'true'
) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crynobone and others added 7 commits November 7, 2023 08:04
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
@crynobone crynobone marked this pull request as ready for review November 7, 2023 06:38
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @crynobone! 👏

@taylorotwell taylorotwell merged commit f517d79 into 4.x Nov 7, 2023
8 checks passed
@taylorotwell taylorotwell deleted the agent-replacement branch November 7, 2023 13:13
@Sarsmooch
Copy link

Sarsmooch commented Nov 16, 2023

I don't know if I could be doing something wrong, but after the update, $session->agent->platform() and $session->agent->browser() return just 1. I created a new empty laravel project to make sure of this, and the same thing happens.

What seems even stranger to me is that during testing, when doing {{ $session->agent->platform() }}, this prints the platform correctly, while {{ $session->agent->platform() ? $session->agent->platform() : __('Unknown') }} prints '1' (true)

@Jared87
Copy link

Jared87 commented Nov 16, 2023

I don't know if I could be doing something wrong, but after the update, $session->agent->platform() and $session->agent->browser() return just 1. I created a new empty laravel project to make sure of this, and the same thing happens.

What seems even stranger to me is that during testing, when doing {{ $session->agent->platform() }}, this prints the platform correctly, while {{ $session->agent->platform() ? $session->agent->platform() : __('Unknown') }} prints '1' (true)

I have the same issue, as i see the problem seems to occur somewhere in the caching, but i cannot say it in detail.
The Agent::retrieveUsingCacheOrResolve() Method has the correct result, but the return value seems to be true.

@Sarsmooch
Copy link

I have the same issue, as i see the problem seems to occur somewhere in the caching, but i cannot say it in detail. The Agent::retrieveUsingCacheOrResolve() Method has the correct result, but the return value seems to be true.

I also looked into this, including the Agent::retrieveUsingCacheOrResolve()` method, I cleared the cache and everything I could, I hosted it on a server in order to check if it could be a problem on my machine, so far I haven't identified the real problem. Maybe later I look for more.

For now, what worked for me was simply modifying the echo to {{ $session->agent->platform() ?: 'Unknown' }}.

@crynobone
Copy link
Member Author

crynobone commented Nov 17, 2023

@Sarsmooch can you submit a new bug report with Browser + Platform + User Agent string so we can debug this further.

e.g:
CleanShot 2023-11-17 at 08 50 16

@Rastian95
Copy link

I don't understand why remove functionalities instead of add new ones!?
There is no more, isPhone(), isRobot(), robot(), deviceType(), device(), languages()
So now we need to extend the class and add these functions from jenssegers/agent? or what do you recommend to do?

@crynobone
Copy link
Member Author

So now we need to extend the class and add these functions from jenssegers/agent? or what do you recommend to do?

Feel free to add class/feature based on your requirements. Agent usage is limited to what's being used within Jetstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants