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

[ENG-6165] bugfixing followup #10746

Merged
Merged
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
6 changes: 2 additions & 4 deletions osf/management/commands/monthly_reporters_go.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,8 @@ def add_arguments(self, parser):
)

def handle(self, *args, **options):
errors = monthly_reporters_go(
monthly_reporters_go(
report_year=getattr(options.get('yearmonth'), 'year', None),
report_month=getattr(options.get('yearmonth'), 'month', None),
)
for error_key, error_val in errors.items():
self.stdout.write(self.style.ERROR(f'error running {error_key}: ') + error_val)
self.stdout.write(self.style.SUCCESS('done.'))
self.stdout.write(self.style.SUCCESS('reporter tasks scheduled.'))
2 changes: 1 addition & 1 deletion osf/metrics/reporters/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def report(
def run_and_record_for_month(self, report_yearmonth: YearMonth) -> None:
reports = self.report(report_yearmonth)
for report in reports:
assert report.report_yearmonth == str(report_yearmonth)
report.report_yearmonth = report_yearmonth
report.save()


Expand Down
7 changes: 5 additions & 2 deletions osf/metrics/reporters/institutional_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@ class _InstiUserReportHelper:
def __post_init__(self):
_affiliation = self.user.get_institution_affiliation(self.institution._id)
self.report = InstitutionalUserReport(
report_yearmonth=self.yearmonth,
institution_id=self.institution._id,
user_id=self.user._id,
user_name=self.user.fullname,
department_name=(_affiliation.sso_department or None),
month_last_login=YearMonth.from_date(self.user.date_last_login),
month_last_login=(
YearMonth.from_date(self.user.date_last_login)
if self.user.date_last_login is not None
else None
),
account_creation_date=YearMonth.from_date(self.user.created),
orcid_id=self.user.get_verified_external_id('ORCID', verified_only=True),
public_project_count=self._public_project_queryset().count(),
Expand Down
1 change: 0 additions & 1 deletion osf/metrics/reporters/spam_count.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ def report(self, report_yearmonth):
next_month = report_yearmonth.next_month()

report = SpamSummaryReport(
report_yearmonth=str(report_yearmonth),
# Node Log entries
node_confirmed_spam=NodeLog.objects.filter(
action=NodeLog.CONFIRM_SPAM,
Expand Down
18 changes: 11 additions & 7 deletions osf/metrics/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Meta:

class YearmonthField(metrics.Date):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs, format='strict_year_month', required=True)
super().__init__(*args, **kwargs, format='strict_year_month')
Copy link
Member

Choose a reason for hiding this comment

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

Minor: This makes a lot of YearMonth fields no longer implicitly (sort of) required. Would it be preferable to default to required=True but allow required=False to be passed in? e.g. with required = kwargs.pop('required', True)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default for elasticsearch_dsl.Field is required=False, and consistency with that seems like a good idea -- there was only one yearmonth field prior to this project (on the MonthlyReport base class), which now has explicit required=True, and the ones added for this project seem fine to make optional


def deserialize(self, data):
if isinstance(data, YearMonth):
Expand All @@ -46,8 +46,10 @@ def deserialize(self, data):
return YearMonth.from_str(data)
elif isinstance(data, (datetime.datetime, datetime.date)):
return YearMonth.from_date(data)
elif data is None:
return None
else:
raise ValueError('unsure how to deserialize "{data}" (of type {type(data)}) to YearMonth')
raise ValueError(f'unsure how to deserialize "{data}" (of type {type(data)}) to YearMonth')

def serialize(self, data):
if isinstance(data, str):
Expand All @@ -56,6 +58,8 @@ def serialize(self, data):
return str(data)
elif isinstance(data, (datetime.datetime, datetime.date)):
return str(YearMonth.from_date(data))
elif data is None:
return None
else:
raise ValueError(f'unsure how to serialize "{data}" (of type {type(data)}) as YYYY-MM')

Expand All @@ -65,7 +69,7 @@ class MonthlyReport(metrics.Metric):
"""
UNIQUE_TOGETHER_FIELDS = ('report_yearmonth',) # override in subclasses for multiple reports per month

report_yearmonth = YearmonthField()
report_yearmonth = YearmonthField(required=True)

class Meta:
abstract = True
Expand Down Expand Up @@ -232,8 +236,8 @@ class InstitutionalUserReport(MonthlyReport):
institution_id = metrics.Keyword()
# user info:
user_id = metrics.Keyword()
user_name = metrics.Text()
department_name = metrics.Text()
user_name = metrics.Keyword()
department_name = metrics.Keyword()
month_last_login = YearmonthField()
account_creation_date = YearmonthField()
orcid_id = metrics.Keyword()
Expand All @@ -243,5 +247,5 @@ class InstitutionalUserReport(MonthlyReport):
public_registration_count = metrics.Integer()
embargoed_registration_count = metrics.Integer()
published_preprint_count = metrics.Integer()
public_file_count = metrics.Integer()
storage_byte_count = metrics.Integer()
public_file_count = metrics.Long()
storage_byte_count = metrics.Long()
Loading