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

WC2-523: [WEB] NFC card field shows 0 for all beneficiaries even when card is assigned #1735

Merged
merged 8 commits into from
Oct 31, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import { Field } from '../types/fields';
import { useGetBeneficiaryTypesDropdown } from './requests';
import { useGetFields } from './useGetFields';

export const useGetBeneficiaryFields = (beneficiary?: Beneficiary) => {
export const useGetBeneficiaryFields = (
beneficiary: Beneficiary | undefined,
) => {
const { formatMessage } = useSafeIntl();

const { data: beneficiaryTypes } = useGetBeneficiaryTypesDropdown();
Expand Down Expand Up @@ -37,7 +39,7 @@ export const useGetBeneficiaryFields = (beneficiary?: Beneficiary) => {
() => [
{
label: formatMessage(MESSAGES.nfcCards),
value: `${beneficiary?.attributes?.nfc_cards ?? 0}`,
value: `${beneficiary?.nfc_cards ?? 0}`,
key: 'nfcCards',
},
{
Expand All @@ -46,7 +48,7 @@ export const useGetBeneficiaryFields = (beneficiary?: Beneficiary) => {
key: 'uuid',
},
],
[beneficiary?.attributes?.nfc_cards, beneficiary?.uuid, formatMessage],
[beneficiary?.nfc_cards, beneficiary?.uuid, formatMessage],
);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ export type Beneficiary = {
duplicates: number[];
latitude?: number;
longitude?: number;
nfc_cards?: number;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type Props = {
showNote?: boolean;
};

export const InstanceFileContent: FunctionComponent<Props> = ({
const InstanceFileContent: FunctionComponent<Props> = ({
instance,
showQuestionKey = true,
showNote = true,
Expand All @@ -34,3 +34,5 @@ export const InstanceFileContent: FunctionComponent<Props> = ({
</ErrorBoundary>
);
};

export default InstanceFileContent;
1 change: 1 addition & 0 deletions hat/assets/js/apps/Iaso/domains/instances/details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const InstanceDetails: FunctionComponent = () => {
const { formatMessage } = useSafeIntl();
const classes: ClassNames = useStyles();
const goBack = useGoBack(baseUrls.instances);

const params = useParamsObject(
baseUrls.instanceDetail,
) as ParamsWithAccountId & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,15 @@ type Props = {
export const LinkToEntity: FunctionComponent<Props> = ({ entity }) => {
const user = useCurrentUser();
const condition =
userHasPermission(ENTITIES, user) && Boolean(entity?.name);
userHasPermission(ENTITIES, user) &&
(Boolean(entity?.name) || Boolean(entity?.id));
const url = `/${baseUrls.entityDetails}/entityId/${entity?.id}`;

return <LinkTo condition={condition} url={url} text={entity?.name} />;
return (
<LinkTo
condition={condition}
url={url}
text={entity?.name || entity?.id.toString()}
/>
);
};
2 changes: 1 addition & 1 deletion hat/assets/js/apps/Iaso/domains/storages/details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const useStyles = makeStyles(theme => ({
export const Details: FunctionComponent = () => {
const params = useParamsObject(
baseUrls.storageDetail,
) as StorageDetailsParams;
) as unknown as StorageDetailsParams;
const goBack = useGoBack(baseUrls.storages);
const { formatMessage } = useSafeIntl();
const { data, isFetching } = useGetStorageLogs(params);
Expand Down
12 changes: 11 additions & 1 deletion iaso/api/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
from time import gmtime, strftime
from typing import Any, List, Union

from iaso.models.storage import StorageDevice, StorageLogEntry
import xlsxwriter # type: ignore
from django.core.paginator import Paginator
from django.db.models import Exists, Max, OuterRef, Q
from django.db.models import Exists, Max, OuterRef, Q, Count
from django.db.models.functions import Coalesce
from django.http import HttpResponse, JsonResponse, StreamingHttpResponse
from django.shortcuts import get_object_or_404
Expand Down Expand Up @@ -51,13 +52,15 @@ class Meta:
"submitter",
"org_unit",
"duplicates",
"nfc_cards",
]

entity_type_name = serializers.SerializerMethodField()
attributes = serializers.SerializerMethodField()
submitter = serializers.SerializerMethodField()
org_unit = serializers.SerializerMethodField()
duplicates = serializers.SerializerMethodField()
nfc_cards = serializers.SerializerMethodField()

def get_attributes(self, entity: Entity):
if entity.attributes:
Expand All @@ -80,6 +83,13 @@ def get_submitter(self, entity: Entity):
def get_duplicates(self, entity: Entity):
return _get_duplicates(entity)

def get_nfc_cards(self, entity: Entity):
nfc_log_entries_count = StorageDevice.objects.filter(entity=entity, type=StorageDevice.NFC).aggregate(
total_logs=Count("log_entries")
)["total_logs"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Looking good to me 👍


return nfc_log_entries_count or 0

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be careful to add this extra query on the EntitySerializer. I also expect the amount of records in StorageLogEntry to grow.
Did you try to do it with a relation and prefetch_related? If it's complicated then it's okay for me to go with it like this.

@staticmethod
def get_entity_type_name(obj: Entity):
return obj.entity_type.name if obj.entity_type else None
Expand Down
5 changes: 3 additions & 2 deletions iaso/api/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,9 @@ def create(self, _, request):
concerned_orgunit = OrgUnit.objects.get(id=log_data["org_unit_id"])

concerned_entity = None
if "entity_id" in log_data and log_data["entity_id"] is not None:
concerned_entity = Entity.objects.get(uuid=log_data["entity_id"])
entity_id = log_data.get("entity_id") or log_data.get("entity_uuid")
if entity_id:
concerned_entity = Entity.objects.get(uuid=entity_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


account = user.iaso_profile.account

Expand Down
2 changes: 1 addition & 1 deletion iaso/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ def as_full_model(self, with_entity=False):
}

if with_entity and self.entity_id:
result["entity"] = self.entity.as_small_dict()
result["entity"] = self.entity.as_small_dict_with_nfc_cards(self)

return result

Expand Down
13 changes: 13 additions & 0 deletions iaso/models/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ class Meta:
def __str__(self):
return f"{self.name}"

def get_nfc_cards(self, instance):
from iaso.models.storage import StorageDevice, StorageLogEntry

nfc_devices = StorageDevice.objects.filter(entity=self, type=StorageDevice.NFC)
nfc_log_entries_count = StorageLogEntry.objects.filter(device__in=nfc_devices, instances=instance).count()

return nfc_log_entries_count

def as_small_dict(self):
return {
"id": self.pk,
Expand All @@ -205,6 +213,11 @@ def as_small_dict(self):
"attributes": self.attributes and self.attributes.as_dict(),
}

def as_small_dict_with_nfc_cards(self, instance):
entity_dict = self.as_small_dict()
entity_dict["nfc_cards"] = self.get_nfc_cards(instance)
return entity_dict

def as_dict(self):
instances = dict()

Expand Down
86 changes: 53 additions & 33 deletions iaso/tests/api/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,56 +132,76 @@ def test_post_log_multiple_logs(self):

self.assertEqual(StorageLogEntry.objects.count(), num_log_storage_before + 2)

def assertBaseNewStorageDeviceCreated(self, storage, expected_id, expected_type="NFC"):
self.assertEqual(storage.customer_chosen_id, "NEW_STORAGE")
self.assertEqual(storage.account, self.yoda.iaso_profile.account)
self.assertEqual(storage.type, expected_type)
self.assertEqual(storage.status, "OK")
self.assertEqual(storage.org_unit, self.org_unit)
self.assertEqual(storage.entity, self.entity)

def assertBaseNewLogEntryCreated(self, log_entry, expected_id, expected_operation, expected_instances):
self.assertEqual(str(log_entry.id), expected_id)
self.assertEqual(log_entry.operation_type, expected_operation)
self.assertEqual(str(log_entry.performed_at), "2022-10-17 10:32:19+00:00")
self.assertEqual(log_entry.performed_by, self.yoda)
self.assertQuerySetEqual(log_entry.instances.all(), expected_instances, ordered=False)
self.assertEqual(log_entry.org_unit, self.org_unit)
self.assertEqual(log_entry.entity, self.entity)

def test_post_log_base_new_storage(self):
"""
Test the base of the POST /api/mobile/storages/log/ endpoint, in the case where the storage device is new.
"""Test POST /api/mobile/storages/log/ endpoint with a new storage device."""
self.client.force_authenticate(self.yoda)

- Status is 201 CREATED
- Correct values added to the database
"""
num_devices_before = StorageDevice.objects.count()

post_body = self._create_post_body_new_storage("entity_id")
response = self.client.post("/api/mobile/storages/logs/", post_body, format="json")

self.assertEqual(response.status_code, 201)
self.assertEqual(StorageDevice.objects.count(), num_devices_before + 1)

the_storage = StorageDevice.objects.latest("id")
self.assertBaseNewStorageDeviceCreated(the_storage, "123e4567-e89b-12d3-a456-426614174000")

the_log_entry = the_storage.log_entries.first()
self.assertBaseNewLogEntryCreated(
the_log_entry, "123e4567-e89b-12d3-a456-426614174000", "WRITE_PROFILE", [self.instance1, self.instance2]
)

def test_post_log_new_storage_entity_uuid(self):
"""Test POST /api/mobile/storages/log/ with a new storage device using entity UUID."""
self.client.force_authenticate(self.yoda)

num_devices_before = StorageDevice.objects.count()

post_body = [
post_body = self._create_post_body_new_storage("entity_uuid")
response = self.client.post("/api/mobile/storages/logs/", post_body, format="json")

self.assertEqual(response.status_code, 201)
self.assertEqual(StorageDevice.objects.count(), num_devices_before + 1)

the_storage = StorageDevice.objects.latest("id")
self.assertBaseNewStorageDeviceCreated(the_storage, "123e4567-e89b-12d3-a456-426614174000")

the_log_entry = the_storage.log_entries.first()
self.assertBaseNewLogEntryCreated(
the_log_entry, "123e4567-e89b-12d3-a456-426614174000", "WRITE_PROFILE", [self.instance1, self.instance2]
)

def _create_post_body_new_storage(self, entity_key):
return [
{
"id": "123e4567-e89b-12d3-a456-426614174000",
"storage_id": "NEW_STORAGE",
"storage_type": "NFC",
"operation_type": "WRITE_PROFILE",
"instances": [self.instance1.uuid, self.instance2.uuid],
"org_unit_id": self.org_unit.id,
"entity_id": self.entity.uuid,
entity_key: self.entity.uuid,
"performed_at": 1666002739.171, # In seconds
}
]
response = self.client.post("/api/mobile/storages/logs/", post_body, format="json")

self.assertEqual(response.status_code, 201)

# Ensure the storage device was created was decent values
self.assertEqual(StorageDevice.objects.count(), num_devices_before + 1)
the_storage = StorageDevice.objects.latest("id")
self.assertEqual(the_storage.customer_chosen_id, "NEW_STORAGE")
self.assertEqual(the_storage.account, self.yoda.iaso_profile.account)
self.assertEqual(the_storage.type, "NFC")
self.assertEqual(the_storage.status, "OK")

# Ensure the log entry was created with decent values
self.assertEqual(the_storage.log_entries.count(), 1)
the_log_entry = the_storage.log_entries.first()
self.assertEqual(str(the_log_entry.id), "123e4567-e89b-12d3-a456-426614174000")
self.assertEqual(the_log_entry.operation_type, "WRITE_PROFILE")
self.assertEqual(str(the_log_entry.performed_at), "2022-10-17 10:32:19+00:00")
self.assertEqual(the_log_entry.performed_by, self.yoda)
self.assertQuerySetEqual(the_log_entry.instances.all(), [self.instance1, self.instance2], ordered=False)
self.assertEqual(the_log_entry.org_unit, self.org_unit)
self.assertEqual(the_log_entry.entity, self.entity)

# The "orgunit" and "entity" fields should also have been set on the storage device itself
self.assertEqual(the_storage.org_unit, self.org_unit)
self.assertEqual(the_storage.entity, self.entity)

def test_post_log_base_existing_storage(self):
"""Similar to test_post_storage_base_new_storage, but the storage device already exists."""
Expand Down
Loading