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

Only test disabled-items-warnings if application is in editable state. #999

Merged
merged 16 commits into from Mar 20, 2019

Conversation

Projects
None yet
3 participants
@foxlynx
Copy link
Member

foxlynx commented Mar 12, 2019

New, updated story with more description: #1002

@@ -765,7 +765,7 @@
(map dynamic-event->event (:dynamic-events app)))
applicant-attributes (:applicant-attributes application)
messages (remove nil?
[(disabled-items-warning (:catalogue-items application)) ; NB: eval this here so we get nil or a warning
[(when (editable? application)(disabled-items-warning (:catalogue-items application))) ; NB: eval this here so we get nil or a warning

This comment has been minimized.

@opqdonut

opqdonut Mar 12, 2019

Collaborator

I'd prefer a line break at )(, but at least add a space :)

This comment has been minimized.

@opqdonut

opqdonut Mar 12, 2019

Collaborator

should the condition actually be "when application is a draft"?

This comment has been minimized.

@foxlynx

foxlynx Mar 12, 2019

Author Member

I thought one can somehow modify returned application still, or does it create a new draft from the returned?

This comment has been minimized.

@opqdonut

opqdonut Mar 12, 2019

Collaborator

oh right, I guess editable is the right condition then

This comment has been minimized.

@foxlynx

foxlynx Mar 12, 2019

Author Member

We are gonna dig a bit deeper tomorrow irl how this feature should be overall.

@Macroz

This comment has been minimized.

Copy link
Collaborator

Macroz commented Mar 12, 2019

Why is this a solution to the problem found in migration tests?

@foxlynx foxlynx force-pushed the fix/approved-application-error branch from 3162e8d to 943b450 Mar 12, 2019

@@ -773,7 +773,8 @@
(map dynamic-event->event (:dynamic-events app)))
applicant-attributes (:applicant-attributes application)
messages (remove nil?
[(disabled-items-warning (:catalogue-items application)) ; NB: eval this here so we get nil or a warning
[(when (editable? application)

This comment has been minimized.

@Macroz

Macroz Mar 13, 2019

Collaborator

This check should be moved inside disabled-items-warning. However, there are also other needs as discussed so let's specify this task a bit better together.

@foxlynx foxlynx changed the title Only test disabled-items-warnings if application is in editable state. WIP: Only test disabled-items-warnings if application is in editable state. Mar 13, 2019

@foxlynx foxlynx force-pushed the fix/approved-application-error branch from 815f535 to 38910a0 Mar 18, 2019

@Macroz Macroz changed the title WIP: Only test disabled-items-warnings if application is in editable state. Only test disabled-items-warnings if application is in editable state. Mar 20, 2019

@Macroz

Macroz approved these changes Mar 20, 2019

@Macroz

This comment has been minimized.

Copy link
Collaborator

Macroz commented Mar 20, 2019

Closes #1002

@Macroz Macroz merged commit 52a4606 into master Mar 20, 2019

6 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: doo Your tests passed on CircleCI!
Details
ci/circleci: ok Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: war Your tests passed on CircleCI!
Details
ci/circleci: without-db Your tests passed on CircleCI!
Details

@Macroz Macroz deleted the fix/approved-application-error branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.