Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

fix(view): Update API views when removing a view #149

Merged
merged 1 commit into from
Nov 22, 2016

Conversation

brasseld
Copy link
Member

@@ -622,13 +622,29 @@ public ImageEntity getPicture(String apiId) {
@Override
public void deleteViewFromAPIs(final String viewId) {
findAll().forEach(api -> {
if (api.getViews()
if (api.getViews() != null && api.getViews()
.removeIf(view -> view.equals(viewId))) {
Copy link
Member

Choose a reason for hiding this comment

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

Not sur the removeIf is necessary here..

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's your stuff guy...

Copy link
Member

Choose a reason for hiding this comment

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

? I mean not anymore necessary with your refactor...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand why but I let you apply necessary changes.

Copy link
Member

Choose a reason for hiding this comment

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

You are doing a remove of view twice : once by the removeIf, and on the remove() method...

}
});
}

private void removeView(String apiId, String viewId) throws TechnicalManagementException {
try {
Optional<Api> optApi = apiRepository.findById(apiId);
Copy link
Member

Choose a reason for hiding this comment

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

Not better to give the api object in parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

no

Copy link
Member

Choose a reason for hiding this comment

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

not sure...

Copy link
Member Author

Choose a reason for hiding this comment

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

so let's do required changes...

@NicolasGeraud NicolasGeraud assigned aelamrani and unassigned aelamrani Nov 21, 2016
@aelamrani aelamrani merged commit 8b2bd35 into master Nov 22, 2016
@aelamrani aelamrani deleted the issue/#317-delete-view branch November 22, 2016 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants