Skip to content

Retrieve client configuration#357

Merged
tomholub merged 7 commits intomasterfrom
feature/issue-276-retrieve-client-configuration
Jun 24, 2021
Merged

Retrieve client configuration#357
tomholub merged 7 commits intomasterfrom
feature/issue-276-retrieve-client-configuration

Conversation

@ekievsky
Copy link
Contributor

@ekievsky ekievsky commented Jun 18, 2021

close #276


Tests :

  • Tests will be added later

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

Added fetching of ClientConfig that includes organisational rules;
Fetching org rules on AppStartup;
@ekievsky ekievsky marked this pull request as draft June 18, 2021 14:25
@tomholub
Copy link
Collaborator

Sorry for the naming confusion earlier, I see we called it DomainRules - let's settle on ClientConfiguration as the way to call this, and refactor based on above comments accordingly. Thanks!

@ekievsky ekievsky marked this pull request as ready for review June 23, 2021 16:23
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Looks good - except for the code blunder

guard let userDomain = email.recipientDomain,
func getClientConfiguration(for email: String) -> Promise<ClientConfiguration?> {
Promise<ClientConfiguration?> { resolve, _ in
guard let userDomain = "name@flowcrypt.com".recipientDomain,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Before marking a PR as ready for review, please do a self-review first on github. I do this for all code I push to GitHub before I ask others to review. This way you ensure that it is really ready for review and that bugs or temporary code was not left in the PR. Thanks!

Copy link
Contributor Author

@ekievsky ekievsky Jun 24, 2021

Choose a reason for hiding this comment

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

You're right, I should've been more careful. Sorry about that. I'll fix that shortly!

@tomholub tomholub marked this pull request as draft June 23, 2021 21:42
@tomholub tomholub marked this pull request as ready for review June 24, 2021 12:47
@tomholub tomholub merged commit a93e296 into master Jun 24, 2021
@tomholub tomholub deleted the feature/issue-276-retrieve-client-configuration branch June 24, 2021 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FES integration to retrieve Client Configuration (OrgRules)

3 participants