Feature/devtooling-1019 Bulk Contact Management within the Contact Lists resource #1506
Conversation
… the contact_list resource
bbbco
requested review from
dginty4,
carnellj-genesys,
HemanthDogiparthi12,
charliecon,
kavinbalagen,
BrianMoyles and
sureshperiyappan
as code owners
| @@ -1,6 +1,10 @@ | |||
| --- | |||
| page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}" | |||
| {{ if gt (len (split .Description "[DEPRECATED]")) 1 -}} | |||
There was a problem hiding this comment.
See https://developer.hashicorp.com/terraform/registry/providers/docs#subcategories for details about subcategory, and I got the logic from hashicorp/terraform-plugin-docs#156 (comment)
| diagErr := util.RetryWhen(util.IsStatus404, func() (*platformclientv2.APIResponse, diag.Diagnostics) { | ||
| resp, err := cp.initiateContactListContactsExport(ctx, contactListId) | ||
| // Sleep one second before attempting to retrieve export url to give the system time to be able to generate the URL | ||
| time.Sleep(time.Second) |
There was a problem hiding this comment.
will this delay be sufficient ?
There was a problem hiding this comment.
Yes, I saw in my testing that trying to retrieve the export url immediately after the export is initiated would not return. 1 second was plenty of time for the URL to be ready.
| @@ -18,30 +17,6 @@ import ( | |||
| "github.com/mypurecloud/platform-client-sdk-go/v150/platformclientv2" | |||
| ) | |||
|
|
|||
| func getAllContacts(ctx context.Context, clientConfig *platformclientv2.Configuration) (resourceExporter.ResourceIDMetaMap, diag.Diagnostics) { | |||
There was a problem hiding this comment.
the regression would fail for these , for someone who has resource_genesyscloud_outbound_contact_list_contact in their export scripts. which forces them to move to the new resource.
There was a problem hiding this comment.
I'm not quite sure what you mean here.
There was a problem hiding this comment.
Hey @HemanthDogiparthi12 if I understand your concern here, you are concerned that a user that has this resource listed as a resource to export in the include filter would not get anything exported and would wonder why. They would have to then move to export the contact list resource itself instead. No other errors would occur.
This is documented in the resource docs. We can add a deprecation notice to the release as well.
Does this address your concerns? Or is there something I may be missing?
| @@ -66,24 +64,14 @@ var ( | |||
| } | |||
| ) | |||
|
|
|||
| func ContactExporter() *resourceExporter.ResourceExporter { | |||
There was a problem hiding this comment.
the regression would fail for these , for someone who has resource_genesyscloud_outbound_contact_list_contact in their export scripts. which forces them to move to the new resource.
There was a problem hiding this comment.
I tested, and if someone has a genesyscloud_outbound_contact_list_contact resource in the filters, it is just ignored.
| CustomizeDiff: customdiff.All( | ||
| customdiff.ComputedIf("contacts_file_content_hash", files.FileContentHashChanged("contacts_filepath", "contacts_file_content_hash")), | ||
| // This function ensures that the contacts file is a CSV file and that it includes the columns defined on the resource | ||
| func(ctx context.Context, d *schema.ResourceDiff, _ interface{}) error { |
There was a problem hiding this comment.
can we include the function definition to a different util class, instead of having it in the schema section.
| if err != nil { | ||
| _, err = url.ParseRequestURI(path) | ||
| if err == nil { | ||
| resp, err := http.Get(path) |
There was a problem hiding this comment.
Had to refactor this function because a non-existent filepath would get to this line and barf with an error complaining about not having a protocol, which was a red herring. The real issue is that the local file didn't exist. I refactored this function so that we check if the path has a protocol and make the call first, then if not, check to ensure the filepath exists. Also added test cases.
bbbco
force-pushed
the
feature/DEVTOOLING-1019v2
branch
from
bfde88c to
97d2c87
Compare
|
@monishapadmavathi confirmed that tests on CI/CD builds are passing for this branch |
|
Resolved merge conflicts |
This PR resolves DEVTOOLING-1019 and makes a major shift away from managing individual contacts as individual resources to consolidating all the contacts in a contact list into a single CSV file that is user managed. These changes use the GC API's export and upload endpoints to enable bulk within the existing
genesyscloud_outbound_contact_listresource and provides a seamless migration path with documentation.Please take extra special note of the schema file, as it includes a number of strategies that I am proposing we continue in future work:
hashattribute to computed and managing the computations ourselves (instead of depending on the user to set the sha calculations)Also, due to the nature of this PR and to prevent confusion and/or duplication when exporting contact resources, I took the liberty of removing the
contact_list_contactresource exporter in favor of this bulk exporter built intocontact_listresource. I added deprecation and migration notices, and added logic to ensure the resource gets tagged in theDeprecatedsubcategory in the rendered docs.I have checked this work against Amazon Q. I also used Amazon Q to help me develop extra tests across a number of different functions and libraries and ensured the tests are passing.