-
Notifications
You must be signed in to change notification settings - Fork 93
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: Update process with same json #1533
Conversation
5fd3b2d
to
556b9f3
Compare
@@ -98,12 +98,18 @@ class DBProcessRepository(val dbConfig: DbConfig, val modelVersion: ProcessingTy | |||
case CustomProcess(mainClass) => (None, Some(mainClass)) | |||
} | |||
|
|||
def normalizeJsonString(jsonString: String) = jsonString.filterNot(_.isWhitespace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's compare parsed json instead (i.e. Json class). Otherwise we will still have possible issues - like changing order of fields in json object etc. I think we can actually pass Json instead of raw string in some places (although I would do large refactor ATM :))
e286144
to
e85f4c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I added two minor comments
user = loggedUser.username, modelVersion = modelVersion.forType(processingType) | ||
) | ||
|
||
//In some reasons in lastVersion.json is string with whitespaces.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"we compare Json representation to ignore formatting differences"?
case (Some(version), _) if isLastVersionContainsSameJson(version, maybeJson) && version.mainClass == maybeMainClass => | ||
Right(None) | ||
case (_, Some(json)) => | ||
normalizeJsonString(json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need this, as we normalize in isLastVersionContainsSameJson anyway?
140921a
to
ebdf315
Compare
ebdf315
to
3a3bd1c
Compare
After moved rename to process properties we can observe that when we change name then system create new version.. In fact we always send rename request and update process request.. If we change 2-3 times name then we can observe that system creates new version of process - but in json we don't see differences.. I debug mode I saw that lastProcessVersion json contains white spaces like
\n
but in BE tests I couldn't reproduce it..This MR fix this situation.