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

ABDM: Log the errors instead of sending them as a response #1821

Merged
merged 7 commits into from
Feb 10, 2024

Conversation

ProCode2
Copy link
Contributor

Proposed Changes

  • Added log for Exception with stack trace.
  • replace Exception object with error string to be sent to client.

Associated Issue

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

@khavinshankar
Copy link
Member

@ProCode2 can you handle this issue (ohcnetwork/care_fe#7000 (comment)) also in this pr.

To replicate the issue, In the front end,

  1. Go to a patient consultation dashboard
  2. Link an Abha number
  3. Go to another patient's consultation dashboard
  4. Link the same Abha number again

That will trigger the error you see in the comment mentioned.

Have a conditional check if the Abha number already exists before creating the Abha number, and return a response with a neat error message.

care/abdm/api/viewsets/auth.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
data/medibase.json Outdated Show resolved Hide resolved
@ProCode2 ProCode2 requested a review from sainak January 18, 2024 08:17
@ProCode2
Copy link
Contributor Author

@ProCode2 can you handle this issue (coronasafe/care_fe#7000 (comment)) also in this pr.

To replicate the issue, In the front end,

  1. Go to a patient consultation dashboard
  2. Link an Abha number
  3. Go to another patient's consultation dashboard
  4. Link the same Abha number again

That will trigger the error you see in the comment mentioned.

Have a conditional check if the Abha number already exists before creating the Abha number, and return a response with a neat error message.

@khavinshankar
by testing on https://care.ohc.network I traced it down to this function thats returning the error:

This creates a ABHA object if not exists and then adds ABHA details to patient.

Since create_abha is only creating the abha object if it does not exist it should be the add_abha_details_to_patient thats throwing error? So we need to not add abha details to a patient if that abha details is already linked with another patient?

Do I understand this correctly?

def confirm_with_mobile_otp(self, request):
        data = request.data

        if ratelimit(request, "confirm_with_mobile_otp", [data["txnId"]]):
            raise CaptchaRequiredException(
                detail={"status": 429, "detail": "Too Many Requests Provide Captcha"},
                code=status.HTTP_429_TOO_MANY_REQUESTS,
            )

        serializer = VerifyOtpRequestPayloadSerializer(data=data)
        serializer.is_valid(raise_exception=True)
        response = HealthIdGateway().confirm_with_mobile_otp(data)
        abha_profile = HealthIdGateway().get_profile(response)

        # have a serializer to verify data of abha_profile
        abha_object = self.create_abha(
            abha_profile,
            {
                "access_token": response["token"],
                "refresh_token": response["refreshToken"],
                "txn_id": data["txnId"],
            },
        )

        if "patientId" in data:
            patient_id = data.pop("patientId")
            allowed_patients = get_patient_queryset(request.user)
            patient_obj = allowed_patients.filter(external_id=patient_id).first()
            if not patient_obj:
                raise ValidationError({"patient": "Not Found"})

            if not self.add_abha_details_to_patient(
                abha_object,
                patient_obj,
            ):
                return Response(
                    {"message": "Failed to add abha Number to the patient"},
                    status=status.HTTP_400_BAD_REQUEST,
                )

        return Response(
            {"id": abha_object.external_id, "abha_profile": abha_profile},
            status=status.HTTP_200_OK,
        )

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (9475f3e) 61.62% compared to head (8335ba9) 61.60%.

Files Patch % Lines
care/abdm/api/viewsets/healthid.py 33.33% 4 Missing ⚠️
care/abdm/api/viewsets/auth.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1821      +/-   ##
==========================================
- Coverage   61.62%   61.60%   -0.02%     
==========================================
  Files         212      212              
  Lines       11664    11674      +10     
  Branches     1655     1655              
==========================================
+ Hits         7188     7192       +4     
- Misses       4180     4186       +6     
  Partials      296      296              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

care/abdm/api/viewsets/auth.py Outdated Show resolved Hide resolved
data/medibase.json Outdated Show resolved Hide resolved
care/abdm/api/viewsets/healthid.py Outdated Show resolved Hide resolved
@vigneshhari vigneshhari merged commit 5ed5c38 into ohcnetwork:master Feb 10, 2024
3 of 5 checks passed
@ProCode2 ProCode2 deleted the chore_log_errors branch February 17, 2024 05:52
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.

6 participants