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

Bugfix/localization 926 #478

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ jobs:
python: 3.7
name: Project linting & migrations
services:
- mysql
- redis-server
env: DB=sqlite
env: DB=mysql
cache:
pip: true
before_install:
- mysql -e 'CREATE DATABASE IF NOT EXISTS zing CHARACTER SET utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci;'
install:
- pip install -r requirements/travis.txt
script:
Expand Down Expand Up @@ -38,11 +41,11 @@ jobs:
- echo -e '-XX:+DisableExplicitGC\n-Djdk.io.permissionsUseCanonicalPath=true\n-Dlog4j.skipJansi=true\n-server\n' | sudo tee -a /etc/elasticsearch/jvm.options
- sudo chown -R elasticsearch:elasticsearch /etc/default/elasticsearch
- sudo systemctl start elasticsearch
- mysql -e 'CREATE DATABASE IF NOT EXISTS zing CHARACTER SET utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci;'
install:
- pip install -r requirements/travis.txt
script:
# Use SQLite to avoid requiring the DB to be setup
- bash -c "DB=sqlite python manage.py build_assets"
- bash -c "python manage.py build_assets"
- py.test --cov-report=term --cov=. -v --durations=25
after_success:
- codecov
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Generated by Django 3.0.5 on 2020-06-10 21:37

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("pootle_statistics", "0005_auto_20200124_0617"),
]

operations = [
migrations.RunSQL(
sql=[
(
"""
CREATE PROCEDURE `insert_submission` ( IN
creation_time DATETIME,
field_arg INT(11),
submitter_id INT(11),
type_arg INT(11),
unit_id INT(11),
store_id INT(11),
translation_project_id INT(11),
old_value longtext,
new_value longtext,
similarity double,
mt_similarity double,
suggestion_id INT(11),
quality_check_id INT(11),
OUT new_submission_id INT(11))

BEGIN
DECLARE total_rows INT DEFAULT 0;

SELECT COUNT(unit_id) INTO total_rows
FROM `pootle_app_submission`
WHERE `pootle_app_submission`.`creation_time` = creation_time AND
`pootle_app_submission`.`field` = field_arg AND
`pootle_app_submission`.`submitter_id` = submitter_id AND
`pootle_app_submission`.`type` = type_arg AND
`pootle_app_submission`.`unit_id` = unit_id AND
`pootle_app_submission`.`new_value` = new_value FOR UPDATE;

IF total_rows = 0 THEN
INSERT INTO `pootle_app_submission` (
`pootle_app_submission`.`creation_time`,
`pootle_app_submission`.`field`,
`pootle_app_submission`.`submitter_id`,
`pootle_app_submission`.`type`,
`pootle_app_submission`.`unit_id`,
`pootle_app_submission`.`store_id`,
`pootle_app_submission`.`translation_project_id`,
`pootle_app_submission`.`old_value`,
`pootle_app_submission`.`new_value`,
`pootle_app_submission`.`similarity`,
`pootle_app_submission`.`mt_similarity`,
`pootle_app_submission`.`suggestion_id`,
`pootle_app_submission`.`quality_check_id` )
VALUES (creation_time, field_arg, submitter_id,
type_arg, unit_id, store_id, translation_project_id,
old_value, new_value, similarity, mt_similarity,
suggestion_id, quality_check_id );
SET new_submission_id = LAST_INSERT_ID();
END IF;
END
"""
)
],
reverse_sql=[("DROP PROCEDURE IF EXISTS `insert_submission`;")],
),
]
37 changes: 34 additions & 3 deletions pootle/apps/pootle_statistics/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
# This file is a part of the Zing project. It is distributed under the GPL3
# or later license. See the LICENSE file for a copy of the license and the
# AUTHORS file for copyright and authorship information.

from django.conf import settings
from django.contrib.auth import get_user_model
from django.db import models
from django.db import DatabaseError, connection, models
from django.db.models import F
from django.template.defaultfilters import truncatechars
from django.utils.functional import cached_property
Expand Down Expand Up @@ -338,7 +337,39 @@ def get_submission_info(self):
return result

def save(self, *args, **kwargs):
super().save(*args, **kwargs)
suggestion_id = self.suggestion.id if self.suggestion else None
quality_check_id = self.quality_check.id if self.quality_check else None
with connection.cursor() as cursor:
resulting_id = 0
cursor.callproc(
"insert_submission",
[
self.creation_time.strftime("%Y-%m-%d %H:%M:%S"),
self.field,
self.submitter.id,
self.type,
self.unit.id,
self.store.id,
self.translation_project.id,
self.old_value,
self.new_value,
self.similarity,
self.mt_similarity,
suggestion_id,
quality_check_id,
resulting_id,
],
)
# get the value from the OUT variable on position 13 for the newly generated id
cursor.execute("SELECT @_insert_submission_13")
id_query = cursor.fetchone()[0]
if id_query is None:
raise DatabaseError(
"Couldn't insert new submission row, might be a duplicate"
)
self.pk = id_query

self.refresh_from_db()

if not self.needs_scorelog():
return
Expand Down
2 changes: 2 additions & 0 deletions pootle/apps/pootle_store/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ def submit(request, unit):
for field, old_value, new_value in form.updated_fields:
if field == SubmissionFields.TARGET and suggestion:
old_value = str(suggestion.target_f)

sub = Submission(
creation_time=current_time,
translation_project=translation_project,
Expand All @@ -611,6 +612,7 @@ def submit(request, unit):
similarity=form.cleaned_data["similarity"],
mt_similarity=form.cleaned_data["mt_similarity"],
)

sub.save()

# Update current unit instance's attributes
Expand Down
4 changes: 4 additions & 0 deletions pootle/settings/90-local.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,13 @@ DATABASES = {
'PORT': %(db_port)s,
# See https://docs.djangoproject.com/en/1.6/topics/db/transactions/
# required for Django 1.6 + sqlite
'CHARSET':'utf8mb4',
'COLLATION':'utf8mb4_unicode_ci',
'ATOMIC_REQUESTS': True,
'TEST': {
'NAME': 'test_zing',
'CHARSET':'utf8mb4',
'COLLATION':'utf8mb4_unicode_ci'
}
}
}
Expand Down
27 changes: 15 additions & 12 deletions pootle/settings/91-travis.conf
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,31 @@ if os.environ.get("TRAVIS"):
DATABASE_BACKEND = os.environ.get("DB")
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.sqlite3',
'NAME': working_path('dbs/zing_travis.db'),
'USER': '',
'ENGINE': 'django.db.backends.mysql',
'NAME': "zing",
'USER': 'travis',
'PASSWORD': '',
'HOST': '',
'PORT': '',
'ATOMIC_REQUESTS': True,
'CHARSET':'utf8mb4',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to override CHARSET and COLLATION now that they are in 90-local.conf.template file? (90-local.conf file will be loaded before 91-travis.conf)

Copy link
Author

Choose a reason for hiding this comment

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

But is Travis loading 90-local.conf.template on the build environment? It seems to me that It should not run it since it has the .template extension

Copy link
Author

Choose a reason for hiding this comment

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

Any input on this Igor? As I stated above It's not clear to me that travis will use the configuration in 90-local.conf.template

'COLLATION':'utf8mb4_unicode_ci',
'TEST': {
'NAME': '',
'CHARSET': 'utf8',
'NAME': 'test_zing',
'CHARSET':'utf8mb4',
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as above.

'COLLATION':'utf8mb4_unicode_ci'
},
"OPTIONS":{
'init_command': "SET sql_mode='STRICT_ALL_TABLES'",
}
}
}

if DATABASE_BACKEND.startswith("mysql"):
DATABASES['default']['ENGINE'] = 'django.db.backends.mysql'
DATABASES['default']['NAME'] = 'zing'
if DATABASE_BACKEND.startswith("sqlite"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain this change? I think with this PR we're moving away from sqlite.

Copy link
Author

Choose a reason for hiding this comment

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

It's to maintain sqlite support for user's that are on it. Thought I should keep it for compatibility

DATABASES['default']['ENGINE'] = 'django.db.backends.sqlite'
DATABASES['default']['NAME'] = working_path('dbs/zing_travis.db')
DATABASES['default']['USER'] = 'travis'
DATABASES['default']['TEST']['COLLATION'] = 'utf8_general_ci'
DATABASES['default']['OPTIONS'] = {
'init_command': "SET sql_mode='STRICT_ALL_TABLES'",
}


SILENCED_SYSTEM_CHECKS = [
'pootle.W004', # Pootle requires a working mail server
Expand Down
9 changes: 1 addition & 8 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,7 @@ def tests_use_db(request):

@pytest.fixture(scope="session")
def tests_use_migration(request, tests_use_db):
return bool(
tests_use_db
and [
item
for item in request.node.items
if item.get_closest_marker("django_migration")
]
)
return True


@pytest.fixture(autouse=True, scope="session")
Expand Down
Loading