[2/2] DXCDT-457: Add members to the auth0_organization data source
#615
Conversation
sergiught
changed the title
auth0_organization data sourceauth0_organization data source
sergiught
force-pushed
the
feature/DXCDT-457-org-members-data-source
branch
from
d051a13 to
c7dd1e8
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feature/DXCDT-457-org-members #615 +/- ##
=================================================================
- Coverage 86.48% 86.45% -0.03%
=================================================================
Files 77 77
Lines 12042 12068 +26
=================================================================
+ Hits 10414 10433 +19
- Misses 1246 1251 +5
- Partials 382 384 +2
|
| @@ -150,6 +161,30 @@ func fetchAllOrganizationConnections(api *management.Management, organizationID | |||
| return foundConnections, nil | |||
| } | |||
|
|
|||
| func fetchAllOrganizationMembers(api *management.Management, organizationID string) ([]string, error) { | |||
There was a problem hiding this comment.
Just thinking through the performance implications of this. If I understand correctly, there may be a large number of members in an organization. In which case, fetching all members could become a penalty if the customer doesn't want to fetch that data. Though I admit that it could be the primary use case of this data source, so the point might be moot.
Not requesting a change necessarily, but want to get your perspective on this and if there are any ways to mitigate.
There was a problem hiding this comment.
This will CAP at 1000 members, as we're using offset pagination instead of checkpoint pagination, so at most we will make 10 requests to get the list of members. We can further tweak this based on user feedback, but for now this seems like a good compromise.
| @@ -110,6 +126,7 @@ func TestAccDataSourceOrganizationByID(t *testing.T) { | |||
| resource.TestCheckResourceAttr("data.auth0_organization.test", "name", fmt.Sprintf("test-%s", testName)), | |||
| resource.TestCheckResourceAttr("data.auth0_organization.test", "connections.#", "1"), | |||
| resource.TestCheckResourceAttrSet("data.auth0_organization.test", "connections.0.connection_id"), | |||
| resource.TestCheckResourceAttr("data.auth0_organization.test", "members.#", "1"), | |||
| ), | |||
There was a problem hiding this comment.
Is there some way to test the new pagination functionality? IMO, it would ideally be in an integration test but I could see that getting a bit gnarly so would accept a unit test too. Thoughts?
There was a problem hiding this comment.
It's intrinsically tested through the members integration tests.
There was a problem hiding this comment.
But only single-paged requests are getting tested and not multi-paged requests, right? That's my only point here.
There was a problem hiding this comment.
To add, I can see this being a bit gnarly to test. Just looking for some assurances that it works, ideally with a test but at the very least some manual verification.
There was a problem hiding this comment.
It works 😄 , you can sleep well tonight. I changed the per page to 1 and had 2 members.
sergiught
force-pushed
the
feature/DXCDT-457-org-members
branch
from
9d1583d to
e3933b9
Compare
sergiught
force-pushed
the
feature/DXCDT-457-org-members-data-source
branch
from
c7dd1e8 to
db98dbd
Compare
sergiught
force-pushed
the
feature/DXCDT-457-org-members
branch
from
e3933b9 to
14c962c
Compare
sergiught
force-pushed
the
feature/DXCDT-457-org-members-data-source
branch
from
db98dbd to
4d45c2c
Compare
sergiught
force-pushed
the
feature/DXCDT-457-org-members-data-source
branch
from
4d45c2c to
ed70341
Compare
🔧 Changes
Add members to the
auth0_organizationdata source.📚 References
auth0_organization_membersresource #614🔬 Testing
📝 Checklist