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

Added API to get country wise skill usage data #811

Merged
merged 8 commits into from
Jun 12, 2018

Conversation

AnupKumarPanwar
Copy link
Member

Fixes
Country wise skill usage analytics #810

Changes:

  1. Created DAO object to store country wise skill usage data in countryWiseSkillUsage.json.
  2. Added updateCountryWiseUsageData() function in SusiCognition.java to update country wise skill usage data of the used skills.
  3. Created GetCountryWiseSkillUsageService.java to act as API to retrieve country wise skill usage data of any skill.

Endpoint :
/cms/getCountryWiseSkillUsage.json

Parameters :

  • model
  • group
  • language
  • skill

Screenshots for the change:

countryWiseSkillUsage.json
image

Sample API call :
/cms/getCountryWiseSkillUsage.json?model=general&group=Knowledge&language=en&skill=ceo

API response
image

Akshat-Jain
Akshat-Jain previously approved these changes Jun 8, 2018
Copy link
Member

@Akshat-Jain Akshat-Jain left a comment

Choose a reason for hiding this comment

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

Please squash the commits. Thanks :-)

anshumanv
anshumanv previously approved these changes Jun 8, 2018
PrP-11
PrP-11 previously approved these changes Jun 10, 2018
sansyrox
sansyrox previously approved these changes Jun 10, 2018
Copy link
Member

@ThatDNS ThatDNS left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts.

@AnupKumarPanwar
Copy link
Member Author

@saurabhjn76 @dynamitechetan @DravitLochan @madhavrathi I think it can be merged now. Please review.

Copy link
Member

@akshatnitd akshatnitd left a comment

Choose a reason for hiding this comment

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

Check inline comments

if (languageName.has(skill_name)) {
JSONArray countryWiseUsageData = languageName.getJSONArray(skill_name);
Boolean countryExists = false;
for (int i = 0; i<countryWiseUsageData.length(); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Add space before and after <

Boolean countryExists = false;
for (int i = 0; i<countryWiseUsageData.length(); i++) {
JSONObject countryUsage = countryWiseUsageData.getJSONObject(i);
if (countryUsage.get("country_code").equals(countryCode)){
Copy link
Member

Choose a reason for hiding this comment

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

Add space before {

JSONObject countryUsage = new JSONObject();
countryUsage.put("country_code", countryCode);
countryUsage.put("country_name", countryName);
countryUsage.put("count", "1");
Copy link
Member

Choose a reason for hiding this comment

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

Do not use String here, instead use integer

countryWiseUsageData.put(countryUsage);
}


Copy link
Member

Choose a reason for hiding this comment

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

Remove extra new lines

modelName.put(group_name, groupName);
skillUsage.put(model_name, modelName, true);
return;

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra new lines

JSONObject countryUsage = new JSONObject();
countryUsage.put("country_code", countryCode);
countryUsage.put("country_name", countryName);
countryUsage.put("count", "1");
Copy link
Member

Choose a reason for hiding this comment

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

Use integer for count

if (modelName.has(group_name)) {
JSONObject groupName = modelName.getJSONObject(group_name);
if (groupName.has(language_name)) {
JSONObject languageName = groupName.getJSONObject(language_name);
Copy link
Member

Choose a reason for hiding this comment

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

Remove extra space after JSONObject

}
}

if (!countryExists) {
Copy link
Member

Choose a reason for hiding this comment

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

It is getting redundant here. You can directly use the block that is present outside the if sections. I would suggest you to take a look at FiveStarRateSkillService.java for reference.

@akshatnitd
Copy link
Member

Also, squash the commits.

Orbiter
Orbiter previously approved these changes Jun 11, 2018
@Orbiter
Copy link
Contributor

Orbiter commented Jun 11, 2018

please resolve conflicts

@AnupKumarPanwar
Copy link
Member Author

@Orbiter Resolved the merge conflicts. Please review.

@AnupKumarPanwar
Copy link
Member Author

@mariobehling @Orbiter @saurabhjn76 @dynamitechetan @DravitLochan @madhavrathi @sudheesh001 Please review. Done the required changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants