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

Add list and detail serializer to facility_asset module #1427

Conversation

GeekGawd
Copy link
Contributor

@GeekGawd GeekGawd commented Jul 4, 2023

Proposed Changes

  • Added List Serializer to only include the necessary fields during List Call of Frontend
  • Added Detail Serializer for detailed view

Associated Issue

Part of ohcnetwork/care_fe#5492

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

@sainak
Copy link
Member

sainak commented Jul 4, 2023

@GeekGawd the changes look good, can you verify and remove the AssetSerializer from other places as well
Note: the AssetSerializer is also used to fetch data here using the public/asset/ endpoint, this looks like it could be handled by the AssetDetailSerializer for this create a new ReadOnly serializer called PublicAssetDetailSerializer with only the field mentioned in the AssetDetail.tsx

and for the tests you can add and validate more data.

Copy link
Member

@sainak sainak left a comment

Choose a reason for hiding this comment

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

changes requested

@GeekGawd
Copy link
Contributor Author

GeekGawd commented Jul 5, 2023

@sainak done please check

@sainak
Copy link
Member

sainak commented Jul 5, 2023

@GeekGawd tests failing

@GeekGawd
Copy link
Contributor Author

GeekGawd commented Jul 5, 2023

@sainak yep fixed it

@sainak
Copy link
Member

sainak commented Jul 5, 2023

Looks good, just add a todo here to remove when remaining serializers are updated @GeekGawd

@sainak
Copy link
Member

sainak commented Jul 20, 2023

@GeekGawd resolve confilicts

@GeekGawd GeekGawd force-pushed the list-detail-serializer-facility-asset branch from f4da1e3 to a582812 Compare July 25, 2023 08:37
@GeekGawd GeekGawd force-pushed the list-detail-serializer-facility-asset branch from 737cd6d to 0cac0c6 Compare July 28, 2023 10:21
@GeekGawd
Copy link
Contributor Author

@sainak merged and fixed test cases

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.

3 participants