-
Notifications
You must be signed in to change notification settings - Fork 1
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: Crowdin proofreader cancelling event is not well triggered - Meeds-io/meeds#2655 #64
base: develop
Are you sure you want to change the base?
fix: Crowdin proofreader cancelling event is not well triggered - Meeds-io/meeds#2655 #64
Conversation
…ds-io/meeds#2655 Currently while cancelling points are removed for the translator, but not for the approver but it should. This pull request intends to make sure that the points are removed for the translator as well
if (trigger.equals(SUGGESTION_DISAPPROVED_TRIGGER)) { | ||
// Retrieve who made that approval from the database | ||
|
||
List<RealizationDTO> realizations = realizationService. | ||
findRealizationsByObjectIdAndObjectType(objectId, TRANSLATION); | ||
|
||
if ( ! realizations.isEmpty()) { | ||
String approvalUsername = realizations.get(0).getEarnerId(); | ||
|
||
eventList.add(new Event(APPROVE_SUGGESTION_EVENT_NAME, | ||
approvalUsername, | ||
approvalUsername, | ||
objectId, | ||
TRANSLATION, | ||
getProjectId(payload), | ||
extractSubItem(payload, TRANSLATION, TARGET_LANGUAGE, ID), | ||
extractSubItem(payload, TRANSLATION, PROVIDER) == null, | ||
extractSubItem(payload, TRANSLATION, STRING, FILE, DIRECTORY_ID), | ||
true, | ||
countWords(extractSubItem(payload, TRANSLATION, STRING, TEXT)))); | ||
} else { | ||
LOG.warn("No realization found while cancelling translation with id: " + objectId); | ||
} |
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 also need to verify the realization event name to check if it is
approveSuggestion
. FYI, It might also besuggestionAdded
, as both share the same objectId and objectType. - There's no need to log a trace when the realization is not found.
- Please ensure the code is formatted properly.
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 a new object type specifically for approvals. Although I'm having trouble identifing where exactly this jacoco-maven build error is coming from
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.
The build failure is caused by a code coverage rule violation: [WARNING] Rule violated for bundle gamification-crowdin-service: instructions covered ratio is 0.26, but expected minimum is 0.27
You need to increase test coverage by adding new unit tests to cover this use case.
I used a new object type for approvals
Currently while cancelling points are removed for the translator, but not for the approver but it should.
This pull request intends to make sure that the points are removed for the translator as well by getting
the previous approval user from the database.