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

55 management of application settings #60

Merged
merged 42 commits into from
Dec 1, 2020

Conversation

rhrt04
Copy link
Contributor

@rhrt04 rhrt04 commented Oct 2, 2020

Fixes #55

Type (Highlight the corresponding type)

  • Bugfix
  • Feature
  • Documentation
  • Refactoring (e.g. Style updates, Test implementation, etc.)
  • Other (please describe):

Checklist

  • Code updated to current masters head
  • Passes CI checks
  • Is a part of an issue
  • Tests added for the bugfix or newly implemented feature, describe below why if not
  • Changelog is updated
  • Documentation of code and features exists

Changes

  • Added management of application settings UI.
  • Updated RoleAndPermissionSeeder.php settings.update and settings.viewAny.
  • Updated SettingPolicy.js addded viewAny and update methods.
  • Added new method in api/v1/ApplicationController updateSettings to update application settings.

Other information

@rhrt04 rhrt04 changed the title Added Setting controller and model, Added Initial view for applicatio… 55-management of application settings Oct 2, 2020
@rhrt04 rhrt04 changed the title 55-management of application settings 55 management of application settings Oct 2, 2020
@rhrt04
Copy link
Contributor Author

rhrt04 commented Oct 2, 2020

Initial Design

image

Updated - 05.10.2020
image

Updated - 20.10.2020
image

@codecov
Copy link

codecov bot commented Oct 2, 2020

Codecov Report

Merging #60 (7f6c4b6) into master (aad604c) will increase coverage by 0.12%.
The diff coverage is 82.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #60      +/-   ##
============================================
+ Coverage     78.87%   79.00%   +0.12%     
- Complexity      316      322       +6     
============================================
  Files           105      108       +3     
  Lines          1671     1762      +91     
  Branches        100      110      +10     
============================================
+ Hits           1318     1392      +74     
- Misses          317      328      +11     
- Partials         36       42       +6     
Impacted Files Coverage Δ Complexity Δ
resources/js/app.js 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
resources/js/components/Room/FileComponent.vue 65.27% <0.00%> (ø) 0.00 <0.00> (ø)
resources/js/views/settings/roles/Index.vue 86.66% <ø> (ø) 0.00 <0.00> (ø)
resources/js/views/settings/users/Index.vue 80.00% <ø> (ø) 0.00 <0.00> (ø)
resources/js/views/settings/Application.vue 74.57% <74.57%> (ø) 0.00 <0.00> (?)
.../Http/Controllers/api/v1/ApplicationController.php 100.00% <100.00%> (ø) 5.00 <3.00> (+3.00)
app/Http/Requests/UpdateSetting.php 100.00% <100.00%> (ø) 1.00 <1.00> (?)
app/Http/Resources/ApplicationSettings.php 100.00% <100.00%> (ø) 2.00 <2.00> (?)
resources/js/policies/SettingPolicy.js 100.00% <100.00%> (ø) 0.00 <0.00> (ø)
resources/js/router.js 83.72% <100.00%> (+0.38%) 0.00 <0.00> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aad604c...7f6c4b6. Read the comment docs.

@SamuelWei
Copy link
Collaborator

SamuelWei commented Oct 2, 2020

Initial Design

image

  • Please remove the explanation texts "Here you can change..."
  • Markenbild -> Logo
  • Add logo preview
  • Most of the settings are not required in this new development, as we are doing many things different than greenlight, please check our config file to see settings that should be configurable via the web gui

@rhrt04 rhrt04 self-assigned this Oct 6, 2020
@rhrt04 rhrt04 requested review from dsst95 and SamuelWei October 6, 2020 12:11
Copy link
Collaborator

@SamuelWei SamuelWei left a comment

Choose a reason for hiding this comment

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

The frontend permissions for managing the application settings are missing

app/Http/Controllers/api/v1/ApplicationController.php Outdated Show resolved Hide resolved
app/Http/Controllers/api/v1/ApplicationController.php Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
app/Http/Requests/UpdateSetting.php Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
tests/Feature/api/v1/SettingsTest.php Outdated Show resolved Hide resolved
@SamuelWei
Copy link
Collaborator

@dennis95stumm I changed a few files and I'm fine with this PR. Could you please also review it, thanks!

Copy link
Contributor

@dsst95 dsst95 left a comment

Choose a reason for hiding this comment

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

I haven't finished the review yet. But here what I've already found.

resources/js/views/settings/roles/Index.vue Outdated Show resolved Hide resolved
tests/Frontend/Router.spec.js Show resolved Hide resolved
app/Http/Requests/UpdateSetting.php Show resolved Hide resolved
app/Http/Requests/UpdateSetting.php Outdated Show resolved Hide resolved
app/Http/Requests/UpdateSetting.php Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/lang/de/settings.js Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
resources/js/views/settings/Application.vue Outdated Show resolved Hide resolved
SamuelWei and others added 7 commits December 1, 2020 12:40
…nt-of-application-settings

# Conflicts:
#	CHANGELOG.md
#	resources/js/lang/de/app.js
#	resources/js/lang/de/settings.js
#	resources/js/lang/en/app.js
#	resources/js/lang/en/settings.js
#	resources/js/router.js
#	resources/js/views/settings/roles/Index.vue
#	tests/Frontend/Router.spec.js
@dsst95 dsst95 merged commit b93cf5b into master Dec 1, 2020
@dsst95 dsst95 deleted the 55-management-of-application-settings branch December 1, 2020 14:52
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.

Management of application settings
3 participants