Skip to content

Closes #3385 After login on public map you can not save#3730

Merged
offtherailz merged 1 commit intogeosolutions-it:masterfrom
kappu72:issue#3385
May 9, 2019
Merged

Closes #3385 After login on public map you can not save#3730
offtherailz merged 1 commit intogeosolutions-it:masterfrom
kappu72:issue#3385

Conversation

@kappu72
Copy link
Copy Markdown
Contributor

@kappu72 kappu72 commented May 7, 2019

Description

Update map permissions after login

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
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
Map permissions aren't updated after login success

What is the new behavior?
Map permissions are updated after login

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

  • Yes, and I documented them in migration notes
  • No

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

Other information:

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 81.409% when pulling 3cbcf62 on kappu72:issue#3385 into d3a2a2a on geosolutions-it:master.

zoomToExtent
} = require('../../actions/map');

const {MAP_INFO_LOAD_START} = require('../../actions/config');
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.

i think we have added the mapstore alias

do for new code we can use it
const {MAP_INFO_LOAD_START} = require('@mapstore/actions/config');

can you change that ?
can you do it also for other requires?

const assign = require('object-assign');
const {Glyphicon} = require('react-bootstrap');
const Message = require('../components/I18N/Message');
const {loadMapInfo} = require('../actions/config');
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 should affect also the component, can you check that?

changeMapView
} = require('../actions/map');
const {mapSelector} = require('../selectors/map');
const {mapSelector, mapIdSelector} = require('../selectors/map');
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.

same here @mapstore/ bla bla bla

@MV88 MV88 changed the title Closes #3385 Closes #3385 After login on public map you can not save May 7, 2019
Copy link
Copy Markdown
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

I'm afraid you didn't solved the issue. please check this action is emitted in the application.

action$.ofType(LOGIN_SUCCESS)
.map(() => {
const mapId = mapIdSelector(getState());
return loadMapInfo(ConfigUtils.getConfigProp("geoStoreUrl") + "extjs/resource/" + mapId, mapId);
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.

Triggering a thunk in an epic do not work as far as I know. loadMapInfo is a thunk.
Are you sure this works?

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.

Since this f1f9a27 thunkmiddleware is applied before redux-observable and so It's possible to dispatch from an epic. https://stackoverflow.com/questions/48510274/use-of-redux-thunk-with-redux-observable

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.

Ok I verified that this issue was present during the first 6 months we used redux-observable, but due to the refactor, this usage is allowed. So I think it can be approved.

@offtherailz offtherailz merged commit 27f0704 into geosolutions-it:master May 9, 2019
@tdipisa
Copy link
Copy Markdown
Member

tdipisa commented May 10, 2019

@kappu72 please backport this to the 2019.01.xx branch.

kappu72 added a commit to kappu72/MapStore2 that referenced this pull request May 10, 2019
offtherailz pushed a commit that referenced this pull request May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After login on public map you can not save

5 participants