add SMTP email sending, LDAP authentication with auto user creation #1119
Conversation
|
Initial setup for working towards issue #1097. This PR adds javascript functions to dynamically hide elements based on selected values in a dropdown menu or if a checkbox is marked. This has no effect as of now but I will make a PR to add the new elements for LDAP authentication which will be hidden by default, unless 'LDAP' is selected in the dav_auth_type options. |
additional options are only visible when they are selected Signed-off-by: Aisha Tammy <[email protected]>
epsilon-0
force-pushed
the
dynamicElements
branch
from
ad78780 to
9ef2ecc
Compare
|
OK, I've added a whole lot of functionality. The code is a bit rough but that can be worked on given everyone is happy with the features provided.
Would really like to get these features in the main repo, I think they are very useful for a lot of people. |
epsilon-0
changed the title
|
This will be easier if the LDAP PR in sabre-io/dav gets merged first, that way the functionality is present in the library and more people can use that. |
|
Thanks for working on this! Baikal already has dynamic element hiding functionality (see for example the database settings page). Could you please use the existing functionality instead of creating a new one? |
|
@epsilon-0 @ByteHamster, I've created a commit in epsilon-0's repository that replaces the dynamic element hiding functionality for the original one. |
…to use a modified version of the old one
|
:O @El-Virus that morphology seems quite complex, kudos for getting that working, I'll try this out for a while on my server and see how it works. |
|
@epsilon-0, Great it's good to check on multiple instances. If you find anything, feel free let me know. |
There was a problem hiding this comment.
I just ran the CI and it found a few problems. Would be great if you could fix them :)
This PR is basically 3 PRs in one (morphology, LDAP, SMTP), so reviewing might take a bit longer than 3 individual PRs would take. The best method to get this merged more quickly would probably be to split it up into 3 individual PRs.
| @@ -24,4 +24,4 @@ function copyToClipboard(el) { | |||
| sel.removeAllRanges(); | |||
| $(el).css({backgroundColor:"#75c753"}); | |||
| $(el).animate({backgroundColor:"transparent"}, 1500); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Please revert the changes to the two files that are now no longer relevant for your PR
|
@ByteHamster, |
This is mainly about reviewing. The morphology change itself can directly be merged with about 5 minutes of review. Both other functions need significantly more review. I had to review many of this kind of PRs in the past and splitting the PR up will definitley help getting this merged more quickly. Over the last month, I have looked at this PR multiple times and then closed it again because my spare time would have been too short for testing both LDAP and SMTP. If it was only one of them, the probability of me actually reviewing would have been a lot higher :) See also https://www.swarmia.com/blog/why-small-pull-requests-are-better/ |
|
@ByteHamster, I'll see what I'm able to do to split the PRs, but about the Morphology commit, its purpose is to hide the SMTP/LDAP fields so it needs to be implemented in both. |
|
The change to |
|
Ok, I'll start making smaller PRs in a few minutes. |
|
@ByteHamster, @epsilon-0, I've opened #1122, #1123, #1124. |
Signed-off-by: Aisha Tammy [email protected]
Closes #1014
Closes #937
Closes #865
Closes #1114