Skip to content

Fix #2965 Authentication Popup#3190

Merged
offtherailz merged 3 commits intogeosolutions-it:masterfrom
allyoucanmap:login-pop
Sep 25, 2018
Merged

Fix #2965 Authentication Popup#3190
offtherailz merged 3 commits intogeosolutions-it:masterfrom
allyoucanmap:login-pop

Conversation

@allyoucanmap
Copy link
Copy Markdown
Contributor

Description

This PR introduces FeedbackMask plugin.
This plugin provides a mask for maps or dashboard not fount or not accessible.

2018-08-28 16_56_11-window

2018-08-28 16_57_14-window

others changes:

  • added actions to handle dashboard error
  • added initMap to MapStore2 in Embedded page to display correctly FeedbackMask in API
  • improved withMask enancher to add custom className props
  • login modal will send to homepage on close action (only for modal triggered by loading error of resource)

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix

What is the current behavior? (You can also link to an open issue here)
Maps or dashborad not found or not accessible don't show clear messages and login modal it's not manage correctly

What is the new behavior?
Added a plugin to show feedbacks for maps/dashboards when they ara a load error

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

It's needed an update in localConfig.json anb plugins.js of projects

Copy link
Copy Markdown
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

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

see if any of my comments are useful

const Message = require('../components/I18N/Message');
const HTML = require('../components/I18N/HTML');

const feedbackMaskSelector = createSelector([
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just a simple note, this can go to its selectors.js file? not sure if you reuse it though

</Button>
);

const MaskBody = emptyState(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this component has no test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MV88 @mbarto @offtherailz This is not a reusable component but related to this plugin.
Where should I move this component to test it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

components/buttons ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an error view like the typical 404 not found page, or access denied.
We could reuse it in the future. For the moment is limited to 2 use cases (dashboards and maps), but it can be refactored quickly (not now, when another use case is required).
@mbarto what do you think about components/security or a new components/errors ?

About the name, instead of MaskBody, something like ResourceUnavailable ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

components/errors is ok, and I agree with the ResourceUnavailable name

@@ -0,0 +1,98 @@
/**
* Copyright 2016, GeoSolutions Sas.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2018

.filter( (action) => action.error && action.error.status === 403 && !isLoggedIn(store.getState()))
.switchMap(() => {
return Rx.Observable.of(setControlProperty('LoginForm', 'enabled', true, true));
return Rx.Observable.of(setControlProperty('LoginForm', 'enabled', true))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a test for this?

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 11, 2018

Coverage Status

Coverage increased (+0.02%) to 81.024% when pulling e80a1d9 on allyoucanmap:login-pop into bbd364d on geosolutions-it:master.

@offtherailz offtherailz merged commit 8c49e9b into geosolutions-it:master Sep 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Authentication Popup

6 participants