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

Fix countersign review. #315

Merged
merged 10 commits into from Dec 1, 2021
Merged

Fix countersign review. #315

merged 10 commits into from Dec 1, 2021

Conversation

alixedi
Copy link
Contributor

@alixedi alixedi commented Dec 1, 2021

A case with e.g. 2 destinations and 2 goods, has 4 distinct advice-subjects.

As a result, when we save approval-all or refuse-all advice, we create 4 advice records - one for each subject and send it to the API.

P.S. This is important and will come in handy when we tackle variable advice.

The flip side to this is that e.g. the countersigner is having to process advice for each subject separately whereas according to the designs, he/she should be able to process the advice from a given advisor as a whole.

What we need to do is to somehow group advice from the same user but different destinations and products into the same block for the purpose of countersigning.

This PR fixes this & hopefully improves on a few other minor things E.g. -

  • I have put in a new template advice_details that does both approvals and refusals. I am hoping that in another PR, I will be able to get rid of approval-advice-details & refusal-advice-details entirely.
  • I have put in a different template for countersign_view. Before this PR, countersign_review was being used for this but looking at the code, it seemed like not much was common.

@alixedi alixedi force-pushed the LTD-1231-fix-countersign-review branch 3 times, most recently from 6cb8ac0 to 72a0c9f Compare December 1, 2021 14:49
@alixedi alixedi marked this pull request as ready for review December 1, 2021 14:52
@alixedi alixedi force-pushed the LTD-1231-fix-countersign-review branch from 72a0c9f to 0644a00 Compare December 1, 2021 14:53
@alixedi alixedi force-pushed the LTD-1231-fix-countersign-review branch from 0644a00 to a0f9503 Compare December 1, 2021 15:00
Copy link
Contributor

@saruniitr saruniitr left a comment

Choose a reason for hiding this comment

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

few comments

@@ -14,6 +14,6 @@
path("countersign/", views.CountersignAdviceView.as_view(), name="countersign_advice_view"),
path("countersign/review-advice/", views.ReviewCountersignView.as_view(), name="countersign_review"),
path("countersign/view-advice/", views.ViewCountersignedAdvice.as_view(), name="countersign_view"),
path("countersign/edit-advice", views.CountersignEditAdviceView.as_view(), name="countersign_edit"),
path("countersign/edit-advice/", views.CountersignEditAdviceView.as_view(), name="countersign_edit"),
Copy link
Contributor

Choose a reason for hiding this comment

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

@wkeeling this is fixed

@alixedi alixedi merged commit 8fe46c3 into master Dec 1, 2021
@alixedi alixedi deleted the LTD-1231-fix-countersign-review branch December 1, 2021 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants