Alan's Update view return version with mod log #1276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://eaflood.atlassian.net/browse/WATER-4634 In this instance we felt it was more constructive to provide the feedback for the original [Update view return version to include mod log info](#1261) proposed PR as a series of commits from where the code has been left. This can be used to guide suggested changes, hopefully with better context than just PR comments.
Cruikshanks
force-pushed
the
update-view-return-version-to-use-modLog-alan
branch
from
b40d2cc to
f626ebf
Compare
I think because of the churn that has gone on with mod logs whilst trying to pull together this PR, changes have been made to the presenter tests that are not really needed. Plus, we can create model instances from an Object, avoiding the need to create 'fake' versions of methods. So, we revert this test file pretty much back to its original state, before then fixing and expanding the tests.
Made me keep putting the `only()` in the wrong place.
I'd overlooked switching to using an actual model instance rather than just an object would mean ensuring we have model instances all the way down!
The notes explain what the scenarios are we need to cover.
We are not looking to retest what is already covered by the model. But we do want to test every scenario our presenter may face. Plus this serves as a handy way to document all the possible outcomes we may get from the presenter.
IMHO I think this cleans up what is happening in the go(). It also avoids doing the same 'work' twice. `$createdBy` gives us a result which it is then on the presenter to determine what to do with it.
I don't think we need this any more. Did some testing and all seemed fine.
Calling a macro appears to be different to a function or filter. Nunjucks interprets the value differently so we don't need to pass it to the `escape` filter.
It was fine as a one liner!
We can revert the beforeEach back to what it was now we have a better idea of how the helper function can work.
We can apply the same tweaks we did to view presenter test to get this passing again with the minimum of changes.
rvsiyad
approved these changes
Aug 21, 2024
Cruikshanks
merged commit f9acbdd
into
update-view-return-version-to-use-modLog
4 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://eaflood.atlassian.net/browse/WATER-4634
In this instance, we felt it was more constructive to provide the feedback for the original Update view return version to include mod log info proposed PR as a series of commits from where the code has been left.
This can be used to guide suggested changes, hopefully with better context than just PR comments.