Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance support for the davmail.outlookUrl property setting (see issue #284) #380

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

esabol
Copy link

@esabol esabol commented Jan 4, 2025

@mguessan : The discussion in issue #284 recently revealed some problems with the davmail.outlookUrl property setting. This PR does the following:

  1. It adds a getOutlookUrl() method to the Settings class.
  2. It corrects the value returned by Settings.getO365Url() when the davmail.outlookUrl property is set. I'm inferring what the return value should be based on the other cases (when the davmail.tld property is set and when neither the davmail.outlookUrl nor the davmail.tld properties are set).
  3. The code was using Settings.OUTLOOK_URL in several places where it should have been referencing the davmail.outlookUrl property instead. These places in the code have been changed to use the new Settings.getOutlookUrl().

@esabol
Copy link
Author

esabol commented Jan 4, 2025

@mguessan : I have also added a second commit to this PR which changes a couple instances of Settings.O365_URL to Settings.getO365Url() in the code.

If you think the instances of Settings.O365_URL in src/test/davmail/http/TestGetRequest.java, and src/test/davmail/http/TestPostRequest.java should also be changed, let me know.

@esabol
Copy link
Author

esabol commented Jan 7, 2025

@mguessan : I've added a third commit based on feedback in issue #284 (comment).

This third commit changes the default value of the davmail.url property to be based on the settings of davmail.tld and/or davmail.outlookUrl, which should mean you no longer need to set davmail.url when either of those other two properties are set.

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.

1 participant