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

forbid changes to legacy project imports [MARXAN-1612] #1136

Conversation

angelhigueraacid
Copy link
Contributor

This PR adds:

  • LegacyProjectImportChecker service. Now block guard checks if a project is blocked depending if there is an ongoing legacy project import for the given project.

Feature relevant tickets

Block legacy projects being imported

@angelhigueraacid angelhigueraacid added the API Everything related the api label Jun 13, 2022
@angelhigueraacid angelhigueraacid self-assigned this Jun 13, 2022
@vercel
Copy link

vercel bot commented Jun 13, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
marxan ✅ Ready (Inspect) Visit Preview Jul 11, 2022 at 8:48AM (UTC)
marxan-storybook ✅ Ready (Inspect) Visit Preview Jul 11, 2022 at 8:48AM (UTC)

@angelhigueraacid
Copy link
Contributor Author

Hi @hotzevzl ! The only thing left that i would like to review about this PR is about using this guard when creating a scenario.
Right now in the create method of api/apps/api/src/modules/scenarios/scenarios.service.ts, we are not doing this.blockGuard.ensureThatScenarioIsNotBlocked(scenario.right.id); so we don't check if any of the blocking operation are being made when creating a new scenario. So,¿Is this the case and we don't want to check if any of these blocking operations are being made? Also, i would say we should at least prevent from creating a scenario when the legacy project import is being executed(on the same project).

@hotzevzl hotzevzl changed the title Feat/marxan 1612 forbid changes to legacy project imports forbid changes to legacy project imports [MARXAN-1612] Jun 27, 2022
Copy link
Member

@hotzevzl hotzevzl left a comment

Choose a reason for hiding this comment

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

thanks @angelhigueraacid - all good, except for some naming that in my opinion could be improved for clarity.

@angelhigueraacid angelhigueraacid force-pushed the feat/MARXAN-1612-forbid-changes-to-legacy-project-imports branch from 0897ca1 to 43ae60f Compare July 11, 2022 08:44
@angelhigueraacid angelhigueraacid merged commit 007d120 into develop Jul 11, 2022
@angelhigueraacid angelhigueraacid deleted the feat/MARXAN-1612-forbid-changes-to-legacy-project-imports branch July 11, 2022 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Everything related the api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants