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.

Sign up for GitHub

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

[settings] New integration settings component #1315

Merged
merged 8 commits into from
Nov 22, 2019
Merged

[settings] New integration settings component #1315

merged 8 commits into from
Nov 22, 2019

Conversation

didierofrivia
Copy link
Member

@didierofrivia didierofrivia commented Oct 9, 2019 •
edited
Loading

What this PR does / why we need it:

The new integration settings page needed a good refactor, it was crafted in a hacky fashion for 2.7 ER1

Which issue(s) this PR fixes

fixes https://issues.jboss.org/browse/THREESCALE-3563

Verification Steps
Apply this patch to make it work:

diff --git a/app/javascript/packs/settings.js b/app/javascript/packs/settings.js
index e9bb1a69..0bfeb4b6 100644
--- a/app/javascript/packs/settings.js
+++ b/app/javascript/packs/settings.js
@@ -1,5 +1,5 @@
-import { initialize as authenticationWidget } from 'Settings/authentication-widget'
+import { initSettings as settingsWidgets } from 'Settings/index'

 document.addEventListener('DOMContentLoaded', () => {
-  authenticationWidget()
+  settingsWidgets({elementId: 'settings'})
 })

TODO

@didierofrivia didierofrivia added enhancement An improvement or enhancement to an existing feature or task APIAP labels Oct 9, 2019
@didierofrivia didierofrivia self-assigned this Oct 9, 2019
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch from 24821c5 to 941707b Compare October 14, 2019 13:05
@codecov
Copy link

codecov bot commented Oct 14, 2019 •
edited
Loading

Codecov Report

Merging #1315 into master will decrease coverage by 5.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1315      +/-   ##
==========================================
- Coverage   90.71%   85.64%   -5.07%     
==========================================
  Files        2319     2080     -239     
  Lines       74200    63987   -10213     
==========================================
- Hits        67309    54803   -12506     
- Misses       6891     9184    +2293
Impacted Files Coverage Δ
lib/tasks/multitenant/tenants.rake 47.16% <0%> (-9.44%) ⬇️
...est/functional/provider/signups_controller_test.rb
...user-management-api/services/mapping_rules_test.rb
...rollers/developer_portal/buyer/stats_controller.rb
...st/workers/delete_account_hierarchy_worker_test.rb
test/integration/sites/forums_controller_test.rb
...presenters/dashboard/top_traffic_presenter_test.rb
lib/developer_portal/lib/liquid/drops/urls.rb
...t/unit/backend_api_logic/service_extension_test.rb
...l/provider/admin/api_docs/specs_controller_test.rb
... and 230 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 37f1984...e1a3fd6. Read the comment docs.

This was referenced Oct 16, 2019
Closed
Merged
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch 17 times, most recently from 6246934 to 7d30235 Compare October 28, 2019 12:28
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch 3 times, most recently from 5f10b37 to 7eea935 Compare October 30, 2019 09:12
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch from 6ee5ee1 to e85f675 Compare October 31, 2019 08:56
josemigallas
josemigallas reviewed Oct 31, 2019
View reviewed changes
app/javascript/src/Settings/components/Common/TypeItemCombo.jsx
legend: string
}

const TypeItemCombo = ({ type, item, legend }: Props) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this component represent, "type item combo"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was crafted to avoid repetition, didn't come up with a better name at the time. It represents a combination of a select + input. The input is a specification of selected value from the former. Any suggestions?

@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch from e85f675 to a850df1 Compare October 31, 2019 10:05
@damianpm damianpm mentioned this pull request Nov 6, 2019
Merged
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch 2 times, most recently from 742441d to fe00b4d Compare November 20, 2019 16:11
didierofrivia and others added 8 commits November 20, 2019 17:23
@didierofrivia
[pf-form] FormFieldset component simplified
c9de4e2 
* Also fixed imports
* And added to flow index
@didierofrivia
[settings] FormSettings types
daf1355
@didierofrivia
[settings] FormSettings defaults
83b3469 
* Used as a template for testing
* Also used for default values while developing
@didierofrivia
[settings] Common components
0a7534c 
* Crafted in order to ease the composition of Settings Form
* Could be re-used for any other form using patterfly-react
* When the time comes and it's used in other modules, should be moved to
a different directory
@didierofrivia
[settings] OIDC Component
6cf06eb
@didierofrivia
[settings] Authentication Settings component
762d796
@didierofrivia
[settings] Settings Form component
5d9e091
@damianpm @didierofrivia
[patternfly] Include PF4 styles for settings page
e1a3fd6
@didierofrivia didierofrivia force-pushed the THREESCALE-3563/integration-settings branch from fe00b4d to e1a3fd6 Compare November 20, 2019 16:23
@didierofrivia didierofrivia changed the title [wip][settings] New integration settings component [settings] New integration settings component Nov 21, 2019
@didierofrivia didierofrivia merged commit 8258631 into master Nov 22, 2019
@didierofrivia didierofrivia deleted the THREESCALE-3563/integration-settings branch November 22, 2019 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guicassolato guicassolato guicassolato left review comments

@damianpm damianpm damianpm approved these changes

@josemigallas josemigallas josemigallas approved these changes

@thomasmaas thomasmaas Awaiting requested review from thomasmaas

Assignees

@didierofrivia didierofrivia

Labels
APIAP enhancement An improvement or enhancement to an existing feature or task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@didierofrivia @guicassolato @josemigallas @damianpm