misc: add category to integration #2332
Conversation
| Integrations::AnrokIntegration.update_all(category: 'tax_provider') # rubocop:disable Rails/SkipsModelValidations | ||
| Integrations::NetsuiteIntegration.update_all(category: 'accounting') # rubocop:disable Rails/SkipsModelValidations | ||
| Integrations::XeroIntegration.update_all(category: 'accounting') # rubocop:disable Rails/SkipsModelValidations | ||
| Integrations::OktaIntegration.update_all(category: 'system') # rubocop:disable Rails/SkipsModelValidations |
There was a problem hiding this comment.
Run it locally on 4000 integrations - took 0.04s
There was a problem hiding this comment.
I'm thinking that if we don't necessarily need the category for BE feature/code at the moment to leave this PR open and lets get back to it later.
Or is it handy to have for some FE stuff? @ansmonjol @keellyp
|
actually I accidentally found more places that can be refactored using the concept of categories, moving this PR to draft and will update |
…zed if now needed
| @@ -106,7 +106,7 @@ def add_on_items | |||
| end | |||
|
|
|||
| def should_sync_credit_note? | |||
| finalized? && customer.integration_customers.accounting_kind.any? { |c| c.integration.sync_credit_notes } | |||
| finalized? & customer.accounting_customer&.integration&.sync_credit_notes | |||
There was a problem hiding this comment.
updated to bitwise and because logical and returns value if it exist instead of true/false
So in our case if second half returns nil,
&& - returns nil for the whole operation
& - returns false for the whole operation,
no matter what first argument was
annvelents
changed the title
app/models/customer.rb
Outdated
| @@ -105,14 +116,7 @@ def preferred_document_locale | |||
| end | |||
|
|
|||
| def provider_customer | |||
There was a problem hiding this comment.
I assume you're keeping this method because it's used all over the place?
There was a problem hiding this comment.
yeah, becasue there are quite a lot of places we're using it 🙈
but actually i think declaring this method as alias will be better 😅
thank you for putting my attention here
annvelents
force-pushed
the
misc-add-category-to-integrations
branch
from
99ceba1
to
a610720
Compare
annvelents
force-pushed
the
misc-add-category-to-integrations
branch
from
77ef290
to
3327258
Compare
@ansmonjol @keellyp Do you think that having category would be handy on the FE side? My feeling regarding this refactoring is that too many things got changed at once and it would require proper QA before getting merged.
|
|
Switching to category may require a refactor on the FE side |
|
@ansmonjol the original reason was to make it easier to show error for integration, because we were going to associate errors with integrations, and when displaying errors having this doesn't seem right 🙈 : |
|
@lovrocolic yes, I see why, will split it 😁 |
Context
We have integrations of different categories, but now define type of an integration is possible only by the name of integrations, so if we know that integration type is NetsuiteIntegration or XeroIntegration, then we can define that this is an accounting integration. Instead we can store category on the integration itself, which will allow us to have more generic components on the FE, find corresponding integration for an error (when error type is "accounting" we will know which integrations we need to look to be set up on a client)
Description
Added string :category to integrations table and serialized it to FE