Add new fields to UserVisit

[mboboc]

This is an optimization to allow filtering by os/browser/device directly in the database.

django-user-visit is very useful in order to track user visits for websites, however, there are 2 main things happening when using this library:

1. The UserVisit table gets big very fast
2. One would like to filter after Device/OS/Browser which is not possible by default inside the database and needs additional parsing steps of the ua_string in Python

Because the user agent is stored as a string and the corresponding parsing functions are implemented in Python, the filtering is fated to be done in Python resulting in poor performance. Also, because the UserVisit table gets populated fast, the waiting times for filtering goes up fast too.

The straightforward solution is to store the string as separate fields inside the database.

[hugorodgerbrown]

Good idea - I've added some basic edit suggestions just to bring it inline with our other libs.

[hugorodgerbrown]

@mboboc thanks for this update - good idea. Once the linting issues are fixed (and feedback applied) will be happy to merge it.

[hugorodgerbrown]

@mboboc just a couple of linting issues to fix - if you have the poetry environment set up you should be able to run pre-commit run --all-files and it'll fix them. (I hope.)

[mboboc]

@hugorodgerbrown Got it, I'll update the code soon (today I hope).

[mboboc]

The linting errors are from the .flake8 file that has inline comments.

flake8 docs:

Following the recommended settings for Python’s configparser, Flake8 does not support inline comments for any of the keys.

In order to reproduce the error locally I had to update the .pre-commit-config.yaml.

django-user-visit/.pre-commit-config.yaml

Lines 19 to 21 in d78ded9

	# Flake8 includes pyflakes, pycodestyle, mccabe, pydocstyle, bandit
	- repo: https://gitlab.com/pycqa/flake8
	rev: 3.9.2

with

# Flake8 includes pyflakes, pycodestyle, mccabe, pydocstyle, bandit
  - repo: https://github.com/PyCQA/flake8
    rev: 6.1.0

• flake8 switched to Github
• I guess the CI uses a more recent version of flake8 that does treat inline comments as errors

Saw you fixed it in #26.

[hugorodgerbrown]

Ah - yes, we're migrating all our repos from flake8 to ruff, so those errors don't matter in the long run. I'll merge your changes in and fix on master.

[hugorodgerbrown]

Although, just looking at this - the migrations need to be recreated, as they are now out of sync with the fields (default, null).

[hugorodgerbrown]

re. flake8 - if you "sync upstream" on your repo that should bring in the new linting?

[mboboc]

After you merge #26 on master, I could bring the changes to this branch. This should solve it.

[mboboc]

@hugorodgerbrown I merged the master branch into this branch, it should be okay now.

[hugorodgerbrown]

@mboboc merged - I'll cut a release in due course - and will add you to the changelog as the author.

[radudum10]

@hugorodgerbrown

What about the existing entries of UserVisits?
The migration should contain an operation that iterates through the existing entries, sets the new fields (browser, device, os) and saves them, shouldn't it?

[hugorodgerbrown]

@hugorodgerbrown

What about the existing entries of UserVisits? The migration should contain an operation that iterates through the existing entries, sets the new fields (browser, device, os) and saves them, shouldn't it?

Hi @radudum10 - yes, these will need backfilling. Our (hard won) experience with large tables and data migrations is that they can easily block a release or lock up a website at release time, and so our pattern for these types of releases is a management command that can a.) be aborted, and b.) be run out of hours.

There is a bunch of work to be done before the release as this is a major one (upgrading Django / Python classifiers), so I'll add in the management command and instructions on the upgrade.

[hugorodgerbrown]

I have released this as v2.0. There is a bug in the management command (:facepalm:) which means you have to run it with the --force flag for it to pick up anything, but I've run it over 1 million rows of real data, and it's working. It'll take time to run. I will fix the management command and release a patch later.