Skip to content

Conversation

@robsliwi
Copy link
Contributor

Sometimes requests come that have a different
MediaKey for a certain tag then we have stored
in the CardAccountMapping.
This commit adds the functionality to find the tag
by searching its UID.

@robsliwi robsliwi added the discussion open for discussion and improvements label Apr 19, 2018
@robsliwi
Copy link
Contributor Author

This commit should solve errors like https://appsignal.com/evopark-gmbh/sites/5769516e01925b18d51d8bf9/incidents/847/samples/5ad85a7acbf15e0319aa1138.
I'm still waiting for feedback from ICA if the request was intended or if it's an error at their side.

Sometimes requests come that have a different
`MediaKey` for a certain tag then we have stored
in the `CardAccountMapping`.
This commit adds the functionality to find the tag
by searching its UID.
@robsliwi robsliwi force-pushed the feature/lookup_rfid_tag_ids branch from 5500208 to 430f331 Compare April 19, 2018 14:48
@@ -0,0 +1,58 @@
# frozen_string_literal: true
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should either be moved into ruby-utils (so that it's available in all repositories) or reside in spec/dummy. Otherwise we have a very unfortunate code duplication here.

def rfid_tag_information
rfid_tag_id = garage_system.card_account_mappings.find_by(card_key: params[:Media][:MediaKey]).rfid_tag_id
rfid_tag_id = garage_system.card_account_mappings.find_by(card_key: params[:Media][:MediaKey])&.rfid_tag_id ||
RfidTag.find_by(uid: RfidTid.new(params[:DriveIn][:Media][:MediaId]).to_s).id
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with this approach is that the specification doesn't guarantee that this will be present or that the media ID will be an RFID TID. Ideally, when running the sync, the card and customer account mappings are only soft-deleted (via acts_as_paranoid) and when finding them here, we then use the with_deleted scope. That way we don't need to make any assumptions about the request.

# frozen_string_literal: true
require 'rfid_tid'

RSpec.describe RfidTid do
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above: the spec shouldn't be duplicated either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion open for discussion and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants