fix: changing date time to date and applying general imrpovements #81
Conversation
eox_nelp/signals/utils.py
Outdated
| # Certificate doesn't have an expiration date, so this is a thing that the client must define. | ||
| "expiration_date": timestamp + timezone.timedelta(days=365), | ||
| "expiration_date": time + timezone.timedelta(days=365), |
There was a problem hiding this comment.
This in future would be manage via settings?
There was a problem hiding this comment.
Or that depend on course finish?
There was a problem hiding this comment.
I removed that in last commit based on the slack comments https://futurexlms.slack.com/archives/C04Q5CSKA8N/p1691590631770909
eox_nelp/signals/utils.py
Outdated
| @@ -40,12 +40,13 @@ def _generate_external_certificate_data(timestamp, certificate_data): | |||
| group_codes = getattr(settings, "EXTERNAL_CERTIFICATES_GROUP_CODES", {}) | |||
| course_id = str(certificate_data.course.course_key) | |||
| extra_info = getattr(user, "extrainfo", None) | |||
| time = timestamp.date() | |||
There was a problem hiding this comment.
Ok no it comes from it call as an arg.
There was a problem hiding this comment.
What is timestamp? isnt a float???
Or which type it is?
There was a problem hiding this comment.
timestamp renamed to time
| @@ -53,13 +52,14 @@ def make_post(self, path, data): | |||
|
|
|||
| response = self.session.post(url=url, json=data) | |||
|
|
|||
| if response.status_code == status.HTTP_200_OK: | |||
| if response.ok: | |||
There was a problem hiding this comment.
I like this change, status less than 400. So different types of 200, up 300...
andrey-canon
force-pushed
the
and/change_date_format
branch
from
ad2fa82
to
91da0ed
Compare
| "id": user["national_id"], | ||
| "id_type": "saudi", | ||
| }, | ||
| "group_code": certificate_data["group_code"], | ||
| "certificate_type": "completion", # What types do we have ? | ||
| "metadata": { | ||
| "degree": certificate_data["grade"], | ||
| "FAIL": certificate_data["is_passing"], | ||
| "FAIL": not certificate_data["is_passing"], |
There was a problem hiding this comment.
Ok that not has lot of logic
| @@ -44,7 +44,7 @@ def _generate_external_certificate_data(time, certificate_data): | |||
| "id": certificate.id, | |||
| "created_at": time.date(), | |||
| "expiration_date": None, | |||
| "grade": int(certificate_data.grade), | |||
| "grade": float(certificate_data.grade) * 100, | |||
There was a problem hiding this comment.
Does certificates receive float ? or This have to be int? Or could be float like 87.8?
There was a problem hiding this comment.
This field was set a CharField, and the current prod implementation shows values between 0 and 1 where 1 is 100% and 0 is 0% so there are values like 0.85
andrey-canon
force-pushed
the
and/change_date_format
branch
from
91da0ed
to
b961cab
Compare
Description
Changes dateTime to date
Considers all the 2xx status codes
Improves response error logs