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

Support query by date_modified field #2009

Merged
merged 21 commits into from
Feb 16, 2021
Merged

Conversation

DavisRayM
Copy link
Contributor

Changes / Features implemented

  • Add _date_modified into the generated Instance JSON
  • Update tests
  • Add created attribute to the data list XML response Instance representations

Steps taken to verify this change does what is intended

  • Updated tests

Side effects of implementing this change

  • Introduces new migrations that regenerate the JSON representation of all un-deleted Instances. Possible chance of downtime while the migration takes place

Closes #2001

@DavisRayM DavisRayM changed the title Support query by date_modified field [WIP] Support query by date_modified field Feb 4, 2021
ivermac
ivermac previously approved these changes Feb 4, 2021
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

Looks good. Quick question, why do we need the migration file?

@@ -389,6 +386,7 @@ def get_full_dict(self, load_existing=True):
self.date_created = submission_time()

doc[SUBMISSION_TIME] = self.date_created.strftime(MONGO_STRFTIME)
doc[DATE_MODIFIED] = self.date_modified.strftime(MONGO_STRFTIME)
Copy link
Member

Choose a reason for hiding this comment

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

We do have this check https://github.com/onaio/onadata/pull/2009/files#diff-98ae21dd41854ad6194bdbca52b97c4d2b0231f6d06b3ee5ee73c4c88a135e27R385-R386 for date_created ensuring it is not null, should we have the same for date_modified?

@DavisRayM
Copy link
Contributor Author

Looks good. Quick question, why do we need the migration file?

It's there so that the JSON is regenerated to enable users to start using the _date_modified filter. The JSON doesn't seem like it's updated till a Submission is edited

@DavisRayM DavisRayM force-pushed the 2001-search-using-last-modified branch from 5f0907d to 9056773 Compare February 4, 2021 14:27
Comment on lines +1 to +25
# pylint: skip-file
# Generated by Django 2.2.16 on 2021-02-02 07:48

from django.db import migrations
from onadata.apps.logger.models.instance import Instance


def regenerate_instance_json(apps, schema_editor):
"""
Regenerate Instance JSON
"""
for inst in Instance.objects.filter(deleted_at__isnull=True):
inst.json = inst.get_full_dict(load_existing=False)
inst.save()


class Migration(migrations.Migration):

dependencies = [
('logger', '0061_auto_20200713_0814'),
]

operations = [
migrations.RunPython(regenerate_instance_json)
]
Copy link
Member

Choose a reason for hiding this comment

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

We will need a strategy to run this in our production environment, other deployments might be okay.

@@ -388,6 +385,12 @@ def get_full_dict(self, load_existing=True):
if not self.date_created:
self.date_created = submission_time()

if not self.date_modified:
doc[DATE_MODIFIED] = ""
Copy link
Member

@ukanga ukanga Feb 4, 2021

Choose a reason for hiding this comment

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

Could we not include the field or use the date_created value, since that would ideally be the same value on the initial creation of the record? The latter approach is what we did for the DATE_CREATED field.

@DavisRayM DavisRayM force-pushed the 2001-search-using-last-modified branch 4 times, most recently from a764dd7 to 31638e0 Compare February 11, 2021 14:05
@DavisRayM DavisRayM changed the title [WIP] Support query by date_modified field Support query by date_modified field Feb 12, 2021
@DavisRayM DavisRayM force-pushed the 2001-search-using-last-modified branch from 108d71a to c813e1f Compare February 12, 2021 12:42
Comment on lines 388 to 389
if not self.date_modified:
doc[DATE_MODIFIED] = self.date_created
Copy link
Member

Choose a reason for hiding this comment

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

Should this be getting, the date format treatment as below or is it handled elsewhere?

doc[DATE_MODIFIED] = self.date_created.strftime(MONGO_STRFTIME)

Comment on lines 388 to 393
if not self.date_modified:
doc[DATE_MODIFIED] = self.date_created.strftime(
MONGO_STRFTIME)
else:
doc[DATE_MODIFIED] = self.date_modified.strftime(
MONGO_STRFTIME)

Copy link
Member

Choose a reason for hiding this comment

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

Simplify

            if not self.date_modified:
                self.date_modified= self.date_created
           
           doc[DATE_MODIFIED] = self.date_modified.strftime(
                    MONGO_STRFTIME)

@DavisRayM DavisRayM force-pushed the 2001-search-using-last-modified branch from 8f2534b to e7f4ef2 Compare February 15, 2021 11:13
@DavisRayM DavisRayM force-pushed the 2001-search-using-last-modified branch from 7479bb7 to c7202b7 Compare February 15, 2021 11:50
@DavisRayM DavisRayM merged commit ca1838a into master Feb 16, 2021
@DavisRayM DavisRayM deleted the 2001-search-using-last-modified branch February 16, 2021 13:59
@DavisRayM DavisRayM mentioned this pull request Feb 23, 2021
1 task
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.

Ability to search by last modified date for both new and edited onadata form instances
3 participants