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.

Sign up for GitHub

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

Jump to bottom

Fix part of #8015 and Fix #9914: Refactor backend-api services to return domain objects. #9917

Merged
merged 15 commits into from
Jul 20, 2020
Merged

Fix part of #8015 and Fix #9914: Refactor backend-api services to return domain objects. #9917

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0b5f937
Topics and Skills Dashboard (All checks passing)
nishantwrp Jul 13, 2020
de976b5
Backend tests
nishantwrp Jul 13, 2020
95bf9bb
All other files done (Tests passing)
nishantwrp Jul 15, 2020
c76eca2
Merge remote-tracking branch 'upstream/develop' into refactor-backend…
nishantwrp Jul 15, 2020
2e26a86
Fix issues with merge
nishantwrp Jul 15, 2020
ba36493
Minor Bug
nishantwrp Jul 16, 2020
fa12ccc
reviews
nishantwrp Jul 16, 2020
3830025
Merge remote-tracking branch 'upstream/develop' into refactor-backend…
nishantwrp Jul 17, 2020
89b955e
reviews
nishantwrp Jul 18, 2020
1404990
reviews
nishantwrp Jul 18, 2020
196dd33
reviews
nishantwrp Jul 18, 2020
50123ab
reviews
nishantwrp Jul 19, 2020
65f528d
reviews
nishantwrp Jul 20, 2020
f63e7a1
Merge remote-tracking branch 'upstream/develop' into refactor-backend…
nishantwrp Jul 20, 2020
4336517
Fix issues with merge
nishantwrp Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions core/controllers/topics_and_skills_dashboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,12 @@ def get(self):
skills_list.append(skill_dict)
categorized_skills_dict[topic.name][
subtopic.title] = skills_list
categorized_skills_dict['untriaged_skills'] = []

for skill_summary_dict in skill_summary_dicts:
skill_id = skill_summary_dict['id']
if (skill_id not in skill_ids_assigned_to_some_topic) and (
skill_id not in merged_skill_ids):
untriaged_skill_summary_dicts.append(skill_summary_dict)
categorized_skills_dict['untriaged_skills'].append({
'skill_id': skill_id,
'skill_description': skill_summary_dict['description']
})
Comment on lines -135 to -138
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this because of two reasons

  • categorized_skills_dict is a dict with topic names as keys. In typescript if there's a dict all the values should be of same type.
  • The same information was already being returned in untriaged_skill_summary_dicts.

Copy link
Member

@seanlip seanlip Jul 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @nishantwrp, adding @Showtim3 for review since he can advise on whether this change makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok @seanlip, Please note that to declare a type for this in the backend-api service. This is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Yup sorry -- I don't disagree. I think @Showtim3 and you would need to collaborate to make sure that the changes to the data structure look good and are propagated correctly. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah the changes look okay. Tested them locally also.

if (skill_id in skill_ids_assigned_to_some_topic) and (
skill_id not in merged_skill_ids):
mergeable_skill_summary_dicts.append(skill_summary_dict)
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/topics_and_skills_dashboard_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_get(self):
if skill_dict['description'] == 'Description 3':
self.assertEqual(skill_dict['id'], self.linked_skill_id)
self.assertEqual(
len(json_response['categorized_skills_dict']), 2)
len(json_response['categorized_skills_dict']), 1)
self.assertEqual(
json_response['untriaged_skill_summary_dicts'][0]['id'],
skill_id)
Expand Down
27 changes: 26 additions & 1 deletion core/templates/components/oppia-angular-root.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ import { AnswerGroupObjectFactory } from
import { AnswerStatsObjectFactory } from
'domain/exploration/AnswerStatsObjectFactory';
import { AppService } from 'services/app.service';
import { AssignedSkillObjectFactory } from
'domain/skill/assigned-skill-object.factory';
import { AudioBarStatusService } from 'services/audio-bar-status.service';
import { AudioFileObjectFactory } from
'domain/utilities/AudioFileObjectFactory';
Expand All @@ -101,6 +103,8 @@ import { AudioTranslationLanguageService } from
'pages/exploration-player-page/services/audio-translation-language.service';
import { AudioTranslationManagerService } from
'pages/exploration-player-page/services/audio-translation-manager.service';
import { AugmentedSkillSummaryObjectFactory } from
'domain/skill/augmented-skill-summary-object.factory';
import { AutogeneratedAudioLanguageObjectFactory } from
'domain/utilities/AutogeneratedAudioLanguageObjectFactory';
import { AutogeneratedAudioPlayerService } from
Expand Down Expand Up @@ -484,6 +488,8 @@ import { SetInputValidationService } from
import { SVMPredictionService } from 'classifiers/svm-prediction.service';
import { SidebarStatusService } from 'domain/sidebar/sidebar-status.service';
import { SiteAnalyticsService } from 'services/site-analytics.service';
import { ShortSkillSummaryObjectFactory } from
'domain/skill/ShortSkillSummaryObjectFactory';
import { SkillCreationBackendApiService } from
'domain/skill/skill-creation-backend-api.service';
import { SkillDifficultyObjectFactory } from
Expand All @@ -500,7 +506,7 @@ import { SkillRightsBackendApiService} from
import { SkillRightsObjectFactory } from
'domain/skill/SkillRightsObjectFactory';
import { SkillSummaryObjectFactory } from
'domain/skill/SkillSummaryObjectFactory';
'domain/skill/skill-summary-object.factory';
import { SolutionObjectFactory } from
'domain/exploration/SolutionObjectFactory';
import { SolutionValidityService } from
Expand Down Expand Up @@ -551,6 +557,8 @@ import { StateStatsObjectFactory } from
'domain/statistics/StateStatsObjectFactory';
import { StateTopAnswersStatsBackendApiService } from
'services/state-top-answers-stats-backend-api.service';
import { StateTopAnswersStatsObjectFactory } from
'domain/statistics/state-top-answers-stats-object.factory';
import { StateWrittenTranslationsService } from
// eslint-disable-next-line max-len
'components/state-editor/state-editor-properties-services/state-written-translations.service';
Expand Down Expand Up @@ -630,6 +638,8 @@ import { UrlService } from 'services/contextual/url.service';
import { UserExplorationPermissionsService } from
'pages/exploration-editor-page/services/user-exploration-permissions.service';
import { UserInfoObjectFactory } from 'domain/user/UserInfoObjectFactory';
import { UserProfileObjectFactory } from
'domain/user/user-profile-object.factory';
import { UtilsService } from 'services/utils.service';
import { ValidatorsService } from 'services/validators.service';
import { VersionTreeService } from
Expand Down Expand Up @@ -673,11 +683,13 @@ export class OppiaAngularRootComponent implements AfterViewInit {
static answerGroupObjectFactory: AnswerGroupObjectFactory;
static answerStatsObjectFactory: AnswerStatsObjectFactory;
static appService: AppService;
static assignedSkillObjectFactory: AssignedSkillObjectFactory;
static audioBarStatusService: AudioBarStatusService;
static audioFileObjectFactory: AudioFileObjectFactory;
static audioLanguageObjectFactory: AudioLanguageObjectFactory;
static audioTranslationLanguageService: AudioTranslationLanguageService;
static audioTranslationManagerService: AudioTranslationManagerService;
static augmentedSkillSummaryObjectFactory: AugmentedSkillSummaryObjectFactory;
static autogeneratedAudioLanguageObjectFactory: AutogeneratedAudioLanguageObjectFactory;
static autogeneratedAudioPlayerService: AutogeneratedAudioPlayerService;
static autoplayedVideosService: AutoplayedVideosService;
Expand Down Expand Up @@ -874,6 +886,7 @@ export class OppiaAngularRootComponent implements AfterViewInit {
static searchExplorationsBackendApiService: SearchExplorationsBackendApiService;
static setInputRulesService: SetInputRulesService;
static setInputValidationService: SetInputValidationService;
static shortSkillSummaryObjectFactory: ShortSkillSummaryObjectFactory;
static sVMPredictionService: SVMPredictionService;
static sidebarStatusService: SidebarStatusService;
static siteAnalyticsService: SiteAnalyticsService;
Expand Down Expand Up @@ -907,6 +920,7 @@ export class OppiaAngularRootComponent implements AfterViewInit {
static stateSolutionService: StateSolutionService;
static stateStatsObjectFactory: StateStatsObjectFactory;
static stateTopAnswersStatsBackendApiService: StateTopAnswersStatsBackendApiService;
static stateTopAnswersStatsObjectFactory: StateTopAnswersStatsObjectFactory;
static stateWrittenTranslationsService: StateWrittenTranslationsService;
static statesObjectFactory: StatesObjectFactory;
static statsReportingBackendApiService: StatsReportingBackendApiService;
Expand Down Expand Up @@ -950,6 +964,7 @@ export class OppiaAngularRootComponent implements AfterViewInit {
static urlService: UrlService;
static userExplorationPermissionsService: UserExplorationPermissionsService;
static userInfoObjectFactory: UserInfoObjectFactory;
static userProfileObjectFactory: UserProfileObjectFactory;
static utilsService: UtilsService;
static validatorsService: ValidatorsService;
static versionTreeService: VersionTreeService;
Expand Down Expand Up @@ -977,11 +992,13 @@ private answerGroupsCacheService: AnswerGroupsCacheService,
private answerGroupObjectFactory: AnswerGroupObjectFactory,
private answerStatsObjectFactory: AnswerStatsObjectFactory,
private appService: AppService,
private assignedSkillObjectFactory: AssignedSkillObjectFactory,
private audioBarStatusService: AudioBarStatusService,
private audioFileObjectFactory: AudioFileObjectFactory,
private audioLanguageObjectFactory: AudioLanguageObjectFactory,
private audioTranslationLanguageService: AudioTranslationLanguageService,
private audioTranslationManagerService: AudioTranslationManagerService,
private augmentedSkillSummaryObjectFactory: AugmentedSkillSummaryObjectFactory,
private autogeneratedAudioLanguageObjectFactory: AutogeneratedAudioLanguageObjectFactory,
private autogeneratedAudioPlayerService: AutogeneratedAudioPlayerService,
private autoplayedVideosService: AutoplayedVideosService,
Expand Down Expand Up @@ -1178,6 +1195,7 @@ private schemaUndefinedLastElementService: SchemaUndefinedLastElementService,
private searchExplorationsBackendApiService: SearchExplorationsBackendApiService,
private setInputRulesService: SetInputRulesService,
private setInputValidationService: SetInputValidationService,
private shortSkillSummaryObjectFactory: ShortSkillSummaryObjectFactory,
private sVMPredictionService: SVMPredictionService,
private sidebarStatusService: SidebarStatusService,
private siteAnalyticsService: SiteAnalyticsService,
Expand Down Expand Up @@ -1211,6 +1229,7 @@ private stateSolicitAnswerDetailsService: StateSolicitAnswerDetailsService,
private stateSolutionService: StateSolutionService,
private stateStatsObjectFactory: StateStatsObjectFactory,
private stateTopAnswersStatsBackendApiService: StateTopAnswersStatsBackendApiService,
private stateTopAnswersStatsObjectFactory: StateTopAnswersStatsObjectFactory,
private stateWrittenTranslationsService: StateWrittenTranslationsService,
private statesObjectFactory: StatesObjectFactory,
private statsReportingBackendApiService: StatsReportingBackendApiService,
Expand Down Expand Up @@ -1254,6 +1273,7 @@ private urlInterpolationService: UrlInterpolationService,
private urlService: UrlService,
private userExplorationPermissionsService: UserExplorationPermissionsService,
private userInfoObjectFactory: UserInfoObjectFactory,
private userProfileObjectFactory: UserProfileObjectFactory,
private utilsService: UtilsService,
private validatorsService: ValidatorsService,
private versionTreeService: VersionTreeService,
Expand Down Expand Up @@ -1282,11 +1302,13 @@ private writtenTranslationsObjectFactory: WrittenTranslationsObjectFactory
OppiaAngularRootComponent.answerGroupObjectFactory = this.answerGroupObjectFactory;
OppiaAngularRootComponent.answerStatsObjectFactory = this.answerStatsObjectFactory;
OppiaAngularRootComponent.appService = this.appService;
OppiaAngularRootComponent.assignedSkillObjectFactory = this.assignedSkillObjectFactory;
OppiaAngularRootComponent.audioBarStatusService = this.audioBarStatusService;
OppiaAngularRootComponent.audioFileObjectFactory = this.audioFileObjectFactory;
OppiaAngularRootComponent.audioLanguageObjectFactory = this.audioLanguageObjectFactory;
OppiaAngularRootComponent.audioTranslationLanguageService = this.audioTranslationLanguageService;
OppiaAngularRootComponent.audioTranslationManagerService = this.audioTranslationManagerService;
OppiaAngularRootComponent.augmentedSkillSummaryObjectFactory = this.augmentedSkillSummaryObjectFactory;
OppiaAngularRootComponent.autogeneratedAudioLanguageObjectFactory = this.autogeneratedAudioLanguageObjectFactory;
OppiaAngularRootComponent.autogeneratedAudioPlayerService = this.autogeneratedAudioPlayerService;
OppiaAngularRootComponent.autoplayedVideosService = this.autoplayedVideosService;
Expand Down Expand Up @@ -1483,6 +1505,7 @@ private writtenTranslationsObjectFactory: WrittenTranslationsObjectFactory
OppiaAngularRootComponent.searchExplorationsBackendApiService = this.searchExplorationsBackendApiService;
OppiaAngularRootComponent.setInputRulesService = this.setInputRulesService;
OppiaAngularRootComponent.setInputValidationService = this.setInputValidationService;
OppiaAngularRootComponent.shortSkillSummaryObjectFactory = this.shortSkillSummaryObjectFactory;
OppiaAngularRootComponent.sVMPredictionService = this.sVMPredictionService;
OppiaAngularRootComponent.sidebarStatusService = this.sidebarStatusService;
OppiaAngularRootComponent.siteAnalyticsService = this.siteAnalyticsService;
Expand Down Expand Up @@ -1515,6 +1538,7 @@ private writtenTranslationsObjectFactory: WrittenTranslationsObjectFactory
OppiaAngularRootComponent.stateSolicitAnswerDetailsService = this.stateSolicitAnswerDetailsService;
OppiaAngularRootComponent.stateSolutionService = this.stateSolutionService;
OppiaAngularRootComponent.stateTopAnswersStatsBackendApiService = this.stateTopAnswersStatsBackendApiService;
OppiaAngularRootComponent.stateTopAnswersStatsObjectFactory = this.stateTopAnswersStatsObjectFactory;
OppiaAngularRootComponent.stateStatsObjectFactory = this.stateStatsObjectFactory;
OppiaAngularRootComponent.stateWrittenTranslationsService = this.stateWrittenTranslationsService;
OppiaAngularRootComponent.statesObjectFactory = this.statesObjectFactory;
Expand Down Expand Up @@ -1559,6 +1583,7 @@ private writtenTranslationsObjectFactory: WrittenTranslationsObjectFactory
OppiaAngularRootComponent.urlService = this.urlService;
OppiaAngularRootComponent.userExplorationPermissionsService = this.userExplorationPermissionsService;
OppiaAngularRootComponent.userInfoObjectFactory = this.userInfoObjectFactory;
OppiaAngularRootComponent.userProfileObjectFactory = this.userProfileObjectFactory;
OppiaAngularRootComponent.utilsService = this.utilsService;
OppiaAngularRootComponent.validatorsService = this.validatorsService;
OppiaAngularRootComponent.versionTreeService = this.versionTreeService;
Expand Down
23 changes: 14 additions & 9 deletions ...s/components/question-directives/modal-templates/question-editor-modal.controller.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Question Editor Modal Controller', function() {
let AlertsService = null;
let QuestionObjectFactory = null;
let QuestionUndoRedoService = null;
let SkillSummaryObjectFactory = null;
let ShortSkillSummaryObjectFactory = null;
let StateEditorService = null;

const associatedSkillSummariesDict = [{
Expand All @@ -44,6 +44,7 @@ describe('Question Editor Modal Controller', function() {
}];
const canEditQuestion = true;
const categorizedSkills = [];
const untriagedSkillSummaries = [];
const groupedSkillSummaries = {
current: [],
others: []
Expand Down Expand Up @@ -72,14 +73,15 @@ describe('Question Editor Modal Controller', function() {
AlertsService = $injector.get('AlertsService');
QuestionObjectFactory = $injector.get('QuestionObjectFactory');
QuestionUndoRedoService = $injector.get('QuestionUndoRedoService');
SkillSummaryObjectFactory = $injector.get('SkillSummaryObjectFactory');
ShortSkillSummaryObjectFactory = $injector.get(
'ShortSkillSummaryObjectFactory');
StateEditorService = $injector.get('StateEditorService');

$uibModalInstance = jasmine.createSpyObj(
'$uibModalInstance', ['close', 'dismiss']);

associatedSkillSummaries = associatedSkillSummariesDict.map(a => (
SkillSummaryObjectFactory.create(a.id, a.description)));
ShortSkillSummaryObjectFactory.create(a.id, a.description)));

question = QuestionObjectFactory.createFromBackendDict({
id: '1',
Expand Down Expand Up @@ -155,6 +157,7 @@ describe('Question Editor Modal Controller', function() {
newQuestionIsBeingCreated: newQuestionIsBeingCreated,
question: question,
questionId: questionId,
untriagedSkillSummaries: untriagedSkillSummaries,
questionStateData: questionStateData,
rubrics: rubrics,
skillNames: skillNames
Expand Down Expand Up @@ -265,7 +268,7 @@ describe('Question Editor Modal Controller', function() {
$scope.$apply();

expect($scope.associatedSkillSummaries).toContain(
SkillSummaryObjectFactory.create(
ShortSkillSummaryObjectFactory.create(
skillSummaryDict.id, skillSummaryDict.description));
expect($scope.associatedSkillSummaries.length).toEqual(4);
expect($scope.getSkillLinkageModificationsArray().length).toBe(1);
Expand Down Expand Up @@ -298,7 +301,7 @@ describe('Question Editor Modal Controller', function() {
$scope.$apply();

expect($scope.associatedSkillSummaries).toContain(
SkillSummaryObjectFactory.create(
ShortSkillSummaryObjectFactory.create(
skillSummaryDict.id, skillSummaryDict.description));

spyOn(QuestionUndoRedoService, 'hasChanges').and.returnValue(false);
Expand All @@ -322,7 +325,7 @@ describe('Question Editor Modal Controller', function() {
$scope.$apply();

expect($scope.associatedSkillSummaries).toContain(
SkillSummaryObjectFactory.create(
ShortSkillSummaryObjectFactory.create(
skillSummaryDict.id, skillSummaryDict.description));

spyOn(QuestionUndoRedoService, 'hasChanges').and.returnValue(true);
Expand Down Expand Up @@ -356,7 +359,7 @@ describe('Question Editor Modal Controller', function() {
$scope.$apply();

expect($scope.associatedSkillSummaries).toContain(
SkillSummaryObjectFactory.create(
ShortSkillSummaryObjectFactory.create(
skillSummaryDict.id, skillSummaryDict.description));

spyOn(QuestionUndoRedoService, 'hasChanges').and.returnValue(true);
Expand Down Expand Up @@ -409,14 +412,15 @@ describe('Question Editor Modal Controller', function() {
AlertsService = $injector.get('AlertsService');
QuestionObjectFactory = $injector.get('QuestionObjectFactory');
QuestionUndoRedoService = $injector.get('QuestionUndoRedoService');
SkillSummaryObjectFactory = $injector.get('SkillSummaryObjectFactory');
ShortSkillSummaryObjectFactory = $injector.get(
'ShortSkillSummaryObjectFactory');
StateEditorService = $injector.get('StateEditorService');

$uibModalInstance = jasmine.createSpyObj(
'$uibModalInstance', ['close', 'dismiss']);

associatedSkillSummaries = associatedSkillSummariesDict.map(a => (
SkillSummaryObjectFactory.create(a.id, a.description)));
ShortSkillSummaryObjectFactory.create(a.id, a.description)));

question = QuestionObjectFactory.createFromBackendDict({
id: '1',
Expand Down Expand Up @@ -494,6 +498,7 @@ describe('Question Editor Modal Controller', function() {
question: question,
questionId: questionId,
questionStateData: questionStateData,
untriagedSkillSummaries: [],
rubrics: rubrics,
skillNames: skillNames
});
Expand Down
Loading