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

[FIX] website : fix authenticate archived visitor #40199

Closed

Conversation

@dbeguin
Copy link
Contributor

dbeguin commented Nov 13, 2019

If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because:

  • _cron_archive_visitors applies on visitor inactive since at least a week
  • there can be only one visitor per partner (sql constraint)
  • The visitor linked to the partner is not retrieved (because archived) and
    we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should :

  • reactivate the previous visitor,
  • copy history from newest to previous one
  • delete the newest one

(last two points were already done before this commit).

This PR also fixes and simplifies the time_statistics computation.
As time_connection_datetime is always set and in the depends, no need to read
values before looping, the data is already fetched in memory. We can than
use directly the value for each visitor in self.

Task ID: 2120464

dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 13, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash. Because the visitor linked to the partner is not retreived
(because archived) and we try to link the partner to a new visitor. The
constraints blocks this. (Only one visitor per partner)

Further than that, if the visitor is archived and the linked partner wants to
loggin again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 6c5520f to 0c82243 Nov 13, 2019
@robodoo robodoo added the seen 🙂 label Nov 13, 2019
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 13, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash. Because the visitor linked to the partner is not retreived
(because archived) and we try to link the partner to a new visitor. The
constraints blocks this. (Only one visitor per partner)

Further than that, if the visitor is archived and the linked partner wants to
loggin again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

This commit also fixes and simpifies the time_statistics computation.
As time_connection_datetime is always set and in the depends, no need to read
values before looping, the data is already fetched in memory. We can than
use directly the value for each visitor in self.

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 0c82243 to 8cc4d93 Nov 13, 2019
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 13, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash. Because the visitor linked to the partner is not retrieved
(because archived) and we try to link the partner to a new visitor. The
constraints blocks this. (Only one visitor per partner)

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

This commit also fixes and simplifies the time_statistics computation.
As time_connection_datetime is always set and in the depends, no need to read
values before looping, the data is already fetched in memory. We can than
use directly the value for each visitor in self.

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 8cc4d93 to dc595c4 Nov 13, 2019
@C3POdoo C3POdoo added the RD label Nov 13, 2019
@robodoo robodoo added the CI 🤖 label Nov 13, 2019
Copy link
Contributor

awa-odoo left a comment

Hi.

Blup.

Bye.

addons/website/models/website_visitor.py Show resolved Hide resolved
visitor_sudo.website_track_ids.write({'visitor_id': partner_visitor.id})
visitor_sudo.unlink()
# Reactivate the older Visitor & Update visitor's Cookie (TODO: check if necessary (maybe done in _handle_webpage_dispatch)

This comment has been minimized.

Copy link
@awa-odoo

awa-odoo Nov 19, 2019

Contributor
  1. I don't like that TODO, can you actually check?
  2. It's unclear here that visitors are archived by a cron, should be explained in the docstring AND in the commit message
  3. I don't see any cookie update, the comment is a little confusing
  4. I attach an image related to all points above

brain

dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 19, 2019
This commit fixes and simplifies the time_statistics computation.
As time_connection_datetime is always set and in the depends, no need to read
values before looping, the data is already fetched in memory. We can than
use directly the value for each visitor in self.

Task ID: 2120464
PR odoo#40199
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 19, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash. Because the visitor linked to the partner is not retrieved
(because archived) and we try to link the partner to a new visitor. The
constraints blocks this. (Only one visitor per partner)

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from dc595c4 to 29a9526 Nov 19, 2019
@robodoo robodoo removed the CI 🤖 label Nov 19, 2019
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 19, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because:
+ _cron_archive_visitors applies on visitor inactive since at least a week
+ there can be only one visitor per partner (sql constraint)
+ The visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 29a9526 to 36dc88b Nov 19, 2019
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 19, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because:
* _cron_archive_visitors applies on visitor inactive since at least a week
* there can be only one visitor per partner (sql constraint)
* The visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should reactivate the previous visitor,
copy history from newest to previous one and delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 36dc88b to b6d70f1 Nov 19, 2019
@robodoo robodoo added the CI 🤖 label Nov 19, 2019
Copy link
Contributor

awa-odoo left a comment

Small readability comment and one remark.

Also, in that commit: "[FIX] website : fix authenticate archived visitor":
You should make bullet points with the second paragraph.
Makes it easier to see what's already done when you say "last two points were already done".

visitor_sudo.website_track_ids.write({'visitor_id': partner_visitor.id})
visitor_sudo.unlink()
# Reactivate the older Visitor

This comment has been minimized.

Copy link
@awa-odoo

awa-odoo Nov 19, 2019

Contributor

# Reactivate the archived visitor (most likely archived by the cron for inactivity reasons)

addons/website/models/res_users.py Show resolved Hide resolved
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 20, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because:
* _cron_archive_visitors applies on visitor inactive since at least a week
* there can be only one visitor per partner (sql constraint)
* The visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should
* reactivate the previous visitor,
* copy history from newest to previous one,
* delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from b6d70f1 to b512fc6 Nov 20, 2019
@robodoo robodoo removed the CI 🤖 label Nov 20, 2019
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 20, 2019
This commit speed up and simplifies the time_statistics computation.
As time_connection_datetime is always set and in the depends, no need to read
values before looping, the data is already fetched in memory. We can than
use directly the value for each visitor in self.

Task ID: 2120464
PR odoo#40199
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Nov 20, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because:
* _cron_archive_visitors applies on visitor inactive since at least a week
* there can be only one visitor per partner (sql constraint)
* The visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should
* reactivate the previous visitor,
* copy history from newest to previous one,
* delete the newest one
(last two points were already done before this commit).

Task ID: 2120464
PR odoo#40199
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from b512fc6 to bf449d1 Nov 20, 2019
@dbeguin

This comment has been minimized.

Copy link
Contributor Author

dbeguin commented Nov 20, 2019

@awa-odoo : tests added for this particular flow.
Comments applied or answered !
Bisous!

@robodoo robodoo added the CI 🤖 label Nov 20, 2019
Copy link
Contributor

awa-odoo left a comment

blork test review

addons/website/tests/test_website_visitor.py Outdated Show resolved Hide resolved
})
partner_demo = self.env.ref('base.partner_demo')
old_visitor.partner_id = partner_demo.id
partner_demo.visitor_ids = [(6, 0, [old_visitor.id])] # TODO DBE : To remove in Master (13.1) after visitor_ids field declaration master fix

This comment has been minimized.

Copy link
@awa-odoo

awa-odoo Nov 21, 2019

Contributor

change to [4, old_visitor.id]

This comment has been minimized.

Copy link
@dbeguin

dbeguin Nov 21, 2019

Author Contributor

I will remove this anyway after forward port (And I need to flush if I use [4, id]) sooooo

addons/website/tests/test_website_visitor.py Outdated Show resolved Hide resolved
addons/website/tests/test_website_visitor.py Outdated Show resolved Hide resolved
if self.target_visitor:
return self.target_visitor
else:
return self.Visitor._create_visitor() if force_create else self.Visitor

This comment has been minimized.

Copy link
@awa-odoo

awa-odoo Nov 21, 2019

Contributor

As you define self.target_visitor just above, can you reach the else case?

This comment has been minimized.

Copy link
@dbeguin

dbeguin Nov 21, 2019

Author Contributor

ah indeed, oopsy (I defined it in the setup originally)

addons/website/tests/test_website_visitor.py Outdated Show resolved Hide resolved
addons/website/tests/test_website_visitor.py Outdated Show resolved Hide resolved
@dbeguin dbeguin force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from 4a3a959 to f8d7439 Nov 21, 2019
@dbeguin

This comment has been minimized.

Copy link
Contributor Author

dbeguin commented Nov 21, 2019

aaaand.. voilà :)

Copy link
Contributor

awa-odoo left a comment

LGTM

dbeguin added 2 commits Nov 19, 2019
This commit fixes the visitor's time_statistics computation. When creating a
new visitor, last connection datetime is not set, so it's not retrieved in the
search_read and crash at search_read_result[visitor.id].

This fix also speeds up and simplifies the time_statistics computation.
As time_connection_datetime is always set (for already created visitor) and in
the depends, no need to read values before looping, the data is already fetched
in memory. We can then use directly the value for each visitor in self.

Task ID: 2120464
PR #40199
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR #40199
Fixes #40077
Fixes #40301
@tde-banana-odoo tde-banana-odoo force-pushed the odoo-dev:13.0-fix-visitor-authenticate-archive-dbe branch from f8d7439 to 33db89b Dec 2, 2019
@tde-banana-odoo

This comment has been minimized.

Copy link
Contributor

tde-banana-odoo commented Dec 2, 2019

@robodoo r+ rebase-ff

@robodoo robodoo added r+ 👌 and removed CI 🤖 labels Dec 2, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Dec 2, 2019

Merge method set to rebase and fast-forward

@robodoo robodoo added the CI 🤖 label Dec 2, 2019
robodoo pushed a commit that referenced this pull request Dec 2, 2019
This commit fixes the visitor's time_statistics computation. When creating a
new visitor, last connection datetime is not set, so it's not retrieved in the
search_read and crash at search_read_result[visitor.id].

This fix also speeds up and simplifies the time_statistics computation.
As time_connection_datetime is always set (for already created visitor) and in
the depends, no need to read values before looping, the data is already fetched
in memory. We can then use directly the value for each visitor in self.

Task ID: 2120464
PR #40199
robodoo pushed a commit that referenced this pull request Dec 2, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR #40199
Fixes #40077
Fixes #40301

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
@robodoo robodoo closed this Dec 2, 2019
@robodoo robodoo deployed to merge Dec 2, 2019 Active
dbeguin added a commit to odoo-dev/odoo that referenced this pull request Dec 3, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR odoo#40199
Fixes odoo#40077
Fixes odoo#40301

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 3, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR #40199
Fixes #40077
Fixes #40301

closes #41220

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 3, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR #40199
Fixes #40077
Fixes #40301

closes #41220

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Dec 3, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR odoo#40199
Fixes odoo#40077
Fixes odoo#40301

Original-signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
X-original-commit: 53a6bed
robodoo pushed a commit that referenced this pull request Dec 4, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR #40199
Fixes #40077
Fixes #40301

closes #41273

Original-signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
X-original-commit: 53a6bed
Signed-off-by: David Beguin <dbeguin@users.noreply.github.com>
bbh-odoo added a commit to odoo-dev/odoo that referenced this pull request Dec 9, 2019
If a visitor logs in (and has a visitor_id in his cookie), the partner will be
linked to the visitor. If, after 1 week, the visitor tries to connect again
with a different session (or another visitor_id in cookies), the authenticate
will crash because

  * _cron_archive_visitors applies on visitor inactive since at least a week
  * there can be only one visitor per partner (sql constraint)
  * the visitor linked to the partner is not retrieved (because archived) and
  we try to link the partner to a new visitor.

Further than that, if the visitor is archived and the linked partner wants to
login again with a new visitor_id, we should

  * reactivate the previous visitor,
  * copy history from newest to previous one,
  * delete the newest one

Note that last two points were already done before this commit.

Task ID: 2120464
PR odoo#40199
Fixes odoo#40077
Fixes odoo#40301

closes odoo#41273

Original-signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
X-original-commit: 53a6bed
Signed-off-by: David Beguin <dbeguin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.