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

Jump to bottom

Show the CORS settings in the UI when enable_general_settings_admin=0 #13660

Merged
merged 3 commits into from Nov 4, 2018
Merged

Show the CORS settings in the UI when enable_general_settings_admin=0 #13660

merged 3 commits into from Nov 4, 2018

Conversation

mattab
Copy link
Member

@mattab mattab commented Nov 1, 2018

The INI setting description does not mean to include CORS:

; By setting this option to 0, the following settings will be hidden and disabled from being set in the UI:
; - "Archiving Settings"
; - "Update settings"
; - "Email server settings"
enable_general_settings_admin = 1

@mattab
Show the CORS settings in the UI when enable_general_settings_admin=0
cffaa42 
The INI setting description does not mean to include CORS:
```ini

; By setting this option to 0, the following settings will be hidden and disabled from being set in the UI:
; - "Archiving Settings"
; - "Update settings"
; - "Email server settings"
enable_general_settings_admin = 1
```
@mattab mattab added Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review labels Nov 1, 2018
@mattab mattab added this to the 3.7.0 milestone Nov 1, 2018
sgiehl
sgiehl approved these changes Nov 1, 2018
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Code looks fine. Just to be sure, this will also activate the possibility to change the trusted hostnames

tsteur
tsteur requested changes Nov 1, 2018
View reviewed changes
Copy link
Member

@tsteur tsteur left a comment

Choose a reason for hiding this comment

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

trusted hostnames should not be writable in this case

@mattab
Copy link
Member Author

mattab commented Nov 1, 2018

@sgiehl would it be possible for you to update the PR so that trusted hosts stay hidden, but CORS are visible in their own section in the General settings?

@sgiehl
Copy link
Member

sgiehl commented Nov 2, 2018

@mattab done.

@tsteur tsteur merged commit b39563c into 3.x-dev Nov 4, 2018
@tsteur tsteur deleted the cors branch November 4, 2018 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsteur tsteur tsteur requested changes

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mattab @sgiehl @tsteur