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 #7176: Remove any from some object factories - III #9370

Merged
merged 2 commits into from
May 28, 2020
Merged

Fix part of #7176: Remove any from some object factories - III #9370

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12e77aa
Remove any from some object factories
nishantwrp May 26, 2020
ed6adaa
Merge remote-tracking branch 'upstream/develop' into any-type-4
nishantwrp May 27, 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
35 changes: 16 additions & 19 deletions core/templates/domain/collection/CollectionNodeObjectFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ import { downgradeInjectable } from '@angular/upgrade/static';
import { Injectable } from '@angular/core';

import { AppConstants } from 'app.constants';
// eslint-disable-next-line max-len
import { IExplorationSummary } from 'core/templates/pages/exploration-player-page/services/exploration-recommendations.service';

export interface ICollectionNodeBackendDict {
'exploration_id'?: string;
'exploration_summary'?: IExplorationSummary;
}

export class CollectionNode {
_explorationId: string;
// TODO(#7165): Replace 'any' with the exact type. This has been typed
// as 'any' since '_explorationSummaryObject' is a dict of varying keys.
_explorationSummaryObject: any;
// TODO(#7176): Replace 'any' with the exact type. This has been kept as
// 'any' because 'collectionNodeBackendObject' is a dict with underscore_cased
// keys which give tslint errors against underscore_casing in favor of
// camelCasing.
constructor(collectionNodeBackendObject: any) {
_explorationSummaryObject: IExplorationSummary;

constructor(collectionNodeBackendObject: ICollectionNodeBackendDict) {
this._explorationId = collectionNodeBackendObject.exploration_id;
this._explorationSummaryObject = cloneDeep(
collectionNodeBackendObject.exploration_summary);
Expand Down Expand Up @@ -78,19 +80,16 @@ export class CollectionNode {
// frontend exploration summary tile displaying. Changes to the returned
// object are not reflected in this domain object. The value returned by
// this function is null if doesExplorationExist() returns false.
// TODO(#7165): Replace 'any' with the exact type. This has been typed
// as 'any' since '_explorationSummaryObject' is a dict of varying keys.
getExplorationSummaryObject(): any {
getExplorationSummaryObject(): IExplorationSummary {
// TODO(bhenning): This should be represented by a
// frontend summary domain object that is also shared with
// the search result and profile pages.
return cloneDeep(this._explorationSummaryObject);
}

// Sets the raw exploration summary object stored within this node.
// TODO(#7165): Replace 'any' with the exact type. This has been typed
// as 'any' since '_explorationSummaryObject' is a dict of varying keys.
setExplorationSummaryObject(explorationSummaryBackendObject: any): any {
setExplorationSummaryObject(
explorationSummaryBackendObject: IExplorationSummary): void {
this._explorationSummaryObject = cloneDeep(
explorationSummaryBackendObject);
}
Expand All @@ -109,11 +108,9 @@ export class CollectionNodeObjectFactory {
// Static class methods. Note that "this" is not available in static
// contexts. This function takes a JSON object which represents a backend
// collection node python dict.
// TODO(#7176): Replace 'any' with the exact type. This has been kept as
// 'any' because 'collectionNodeBackendObject' is a dict with underscore_cased
// keys which give tslint errors against underscore_casing in favor of
// camelCasing.
create(collectionNodeBackendObject: any): CollectionNode {
create(
collectionNodeBackendObject: ICollectionNodeBackendDict):
CollectionNode {
return new CollectionNode(collectionNodeBackendObject);
}

Expand Down
32 changes: 20 additions & 12 deletions core/templates/domain/collection/CollectionObjectFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,22 @@ import cloneDeep from 'lodash/cloneDeep';
import { downgradeInjectable } from '@angular/upgrade/static';
import { Injectable } from '@angular/core';

import { CollectionNode, CollectionNodeObjectFactory } from
'domain/collection/CollectionNodeObjectFactory';
import {
ICollectionNodeBackendDict,
CollectionNode,
CollectionNodeObjectFactory
} from 'domain/collection/CollectionNodeObjectFactory';

interface ICollectionBackendDict {
'id'?: string;
'title'?: string;
'objective'?: string;
'language_code'?: string;
'tags'?: string[];
'category'?: string;
'version'?: number;
'nodes'?: ICollectionNodeBackendDict[];
}

export class Collection {
_id: string;
Expand All @@ -35,12 +49,9 @@ export class Collection {
_version: number;
_nodes: CollectionNode[];
_explorationIdToNodeIndexMap: {};
// TODO(#7176): Replace 'any' with the exact type. This has been kept as
// 'any' because 'collectionBackendObject' is a dict with underscore_cased
// keys which give tslint errors against underscore_casing in favor of
// camelCasing.

constructor(
collectionBackendObject: any,
collectionBackendObject: ICollectionBackendDict,
collectionNodeObjectFactory: CollectionNodeObjectFactory) {
this._id = collectionBackendObject.id;
this._title = collectionBackendObject.title;
Expand Down Expand Up @@ -258,11 +269,8 @@ export class Collection {
export class CollectionObjectFactory {
constructor(
private collectionNodeObjectFactory: CollectionNodeObjectFactory) {}
// TODO(#7176): Replace 'any' with the exact type. This has been kept as
// 'any' because 'collectionBackendObject' is a dict with underscore_cased
// keys which give tslint errors against underscore_casing in favor of
// camelCasing.
create(collectionBackendObject: any): Collection {

create(collectionBackendObject: ICollectionBackendDict): Collection {
return new Collection(
collectionBackendObject, this.collectionNodeObjectFactory);
}
Expand Down
2 changes: 1 addition & 1 deletion core/templates/domain/collection/CollectionObjectFactorySpec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ describe('Collection object factory', () => {
});
secondCollection.addCollectionNode(collectionNodeObjectFactory.create({
exploration_id: 'exp_id5',
exploration: {}
exploration_summary: {}
}));

_addCollectionNode('exp_id0');
Expand Down
2 changes: 1 addition & 1 deletion core/templates/domain/collection/guest-collection-progress.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('Guest collection progress service', () => {
title: title,
objective: 'an objective',
category: 'a category',
version: '1',
version: 1,
nodes: []
};
return collectionObjectFactory.create(collectionBackendObject);
Expand Down
99 changes: 52 additions & 47 deletions core/templates/domain/editor/undo_redo/ChangeObjectFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,69 +26,80 @@ import cloneDeep from 'lodash/cloneDeep';
import { downgradeInjectable } from '@angular/upgrade/static';
import { Injectable } from '@angular/core';

import { IMisconceptionBackendDict } from
'domain/skill/MisconceptionObjectFactory';

interface IBackendChangeObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you verified that all the new structures editors work as well?

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 don't think there should be problems on the behaviour with just adding the types, as the code is transpiled to js anyway.

// This interface may not have all the possible values.
// Add properties with a '?' if you find any that needs to be here.
'cmd'?: string;
'property_name'?: string;

'state_name'?: string;
'old_state_name'?: string;
'new_state_name'?: string;

'new_value'?: Object;
'old_value'?: Object;

'exploration_id'?: string;

'new_misconception_dict'?: IMisconceptionBackendDict;
'misconception_id'?: string;

'skill_id'?: string;
'uncategorized_skill_id'?: string;
'new_uncategorized_skill_id'?: string;
'skill_difficulty'?: number;

'explanations'?: string[];
'difficulty'?: string;

'node_id'?: string;

'story_id'?: string;

'subtopic_id'?: number;
'new_subtopic_id'?: number;
'old_subtopic_id'?: number;

'version_number'?: number;
}

export class Change {
// TODO(#7176): Replace 'any' with the exact type. This (and elsewhere
// throughout) has been kept as 'any' because 'backendChangeObject' is a
// dict with possible underscore_cased keys which give tslint errors
// against underscore_casing in favor of camelCasing. Also,
// 'applyChangeToObject' and 'reverseChangeToObject' are functions whose
// return type depends upon the arguments passed to the constructor.
_backendChangeObject: any;
_applyChangeToObject: any;
_reverseChangeToObject: any;
_backendChangeObject: IBackendChangeObject;
_applyChangeToObject: Function;
_reverseChangeToObject: Function;

constructor(
backendChangeObject: any, applyChangeToObject: any,
reverseChangeToObject: any) {
backendChangeObject: IBackendChangeObject, applyChangeToObject: Function,
reverseChangeToObject: Function) {
this._backendChangeObject = cloneDeep(backendChangeObject);
this._applyChangeToObject = applyChangeToObject;
this._reverseChangeToObject = reverseChangeToObject;
}

// Returns the JSON object which represents a backend python dict of this
// change. Changes to this object are not reflected in this domain object.
// TODO(#7176): Replace 'any' with the exact type. This (and elsewhere
// throughout) has been kept as 'any' because 'backendChangeObject' is a
// dict with possible underscore_cased keys which give tslint errors
// against underscore_casing in favor of camelCasing. Also,
// 'applyChangeToObject' and 'reverseChangeToObject' are functions whose
// return type depends upon the arguments passed to the constructor.
getBackendChangeObject(): any {
getBackendChangeObject(): IBackendChangeObject {
return cloneDeep(this._backendChangeObject);
}

// TODO(#7176): Replace 'any' with the exact type. This (and elsewhere
// throughout) has been kept as 'any' because 'backendChangeObject' is a
// dict with possible underscore_cased keys which give tslint errors
// against underscore_casing in favor of camelCasing. Also,
// 'applyChangeToObject' and 'reverseChangeToObject' are functions whose
// return type depends upon the arguments passed to the constructor.
setBackendChangeObject(backendChangeObject: any): any {
setBackendChangeObject(
backendChangeObject: IBackendChangeObject): IBackendChangeObject {
return this._backendChangeObject = cloneDeep(backendChangeObject);
}

// Applies this change to the related object (such as a frontend collection
// domain object).
// TODO(#7176): Replace 'any' with the exact type. This (and elsewhere
// throughout) has been kept as 'any' because 'backendChangeObject' is a
// dict with possible underscore_cased keys which give tslint errors
// against underscore_casing in favor of camelCasing. Also,
// 'applyChangeToObject' and 'reverseChangeToObject' are functions whose
// return type depends upon the arguments passed to the constructor.
applyChange(domainObject: any): void {
applyChange(domainObject: IBackendChangeObject): void {
this._applyChangeToObject(this._backendChangeObject, domainObject);
}

// Reverse-applies this change to the related object (such as a frontend
// collection domain object). This method should only be used to reverse a
// change that was previously applied by calling the applyChange() method.
// TODO(#7176): Replace 'any' with the exact type. This (and elsewhere
// throughout) has been kept as 'any' because 'backendChangeObject' is a
// dict with possible underscore_cased keys which give tslint errors
// against underscore_casing in favor of camelCasing. Also,
// 'applyChangeToObject' and 'reverseChangeToObject' are functions whose
// return type depends upon the arguments passed to the constructor.
reverseChange(domainObject: any): void {
reverseChange(domainObject: IBackendChangeObject): void {
this._reverseChangeToObject(this._backendChangeObject, domainObject);
}
}
Expand All @@ -105,15 +116,9 @@ export class ChangeObjectFactory {
// parameter is a callback which behaves in the same way as the second
// parameter and takes the same inputs, except it should reverse the change
// for the provided domain object.
// TODO(#7176): Replace 'any' with the exact type. This has been kept as
// 'any' because 'backendChangeObject' is a dict with possible
// underscore_cased keys which give tslint errors against underscore_casing
// in favor of camelCasing. Also, 'applyChangeToObject' and
// 'reverseChangeToObject' are functions whose return type depends upon the
// arguments passed to the constructor.
create(
backendChangeObject: any, applyChangeToObject: any,
reverseChangeToObject: any): Change {
backendChangeObject: IBackendChangeObject, applyChangeToObject: Function,
reverseChangeToObject: Function): Change {
return new Change(
backendChangeObject, applyChangeToObject, reverseChangeToObject);
}
Expand Down
4 changes: 2 additions & 2 deletions core/templates/domain/editor/undo_redo/ChangeObjectFactorySpec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Factory for Change domain objects', () => {
backendChangeObject, applyFunc, reverseFunc);

const fakeDomainObject = {
domain_property_name: 'fake value'
property_name: 'fake value'
};
changeDomainObject.applyChange(fakeDomainObject);

Expand All @@ -70,7 +70,7 @@ describe('Factory for Change domain objects', () => {
backendChangeObject, applyFunc, reverseFunc);

const fakeDomainObject = {
domain_property_name: 'fake value'
property_name: 'fake value'
};
changeDomainObject.reverseChange(fakeDomainObject);

Expand Down
8 changes: 4 additions & 4 deletions core/templates/domain/skill/MisconceptionObjectFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
*/

export interface IMisconceptionBackendDict {
feedback: string;
id: string;
'feedback': string;
'id': string;
'must_be_addressed': boolean;
name: string;
notes: string;
'name': string;
'notes': string;
}

import { downgradeInjectable } from '@angular/upgrade/static';
Expand Down
35 changes: 15 additions & 20 deletions core/templates/pages/exploration-player-page/services/exploration-recommendations.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,21 @@ import { ContextService } from 'services/context.service';
import { ServicesConstants } from 'services/services.constants';
import { UrlService } from 'services/contextual/url.service';

export interface ISummary {
category: string;
// eslint-disable-next-line camelcase
community_owned: boolean;
id: string;
// eslint-disable-next-line camelcase
language_code: string;
// eslint-disable-next-line camelcase
num_views: number;
objective: string;
status: string;
tags: string[];
// eslint-disable-next-line camelcase
thumbnail_bg_color: string;
// eslint-disable-next-line camelcase
thumbnail_icon_url: string;
title: string;
export interface IExplorationSummary {
'category'?: string;
'community_owned'?: boolean;
'id'?: string;
'language_code'?: string;
'num_views'?: number;
'objective'?: string;
'status'?: string;
'tags'?: string[];
'thumbnail_bg_color'?: string;
'thumbnail_icon_url'?: string;
'title'?: string;
}
export interface ISummaries {
summaries: ISummary[];
export interface IExplorationSummaries {
summaries: IExplorationSummary[];
}
type RecommendationsUrlParams = {
/* eslint-disable camelcase */
Expand Down Expand Up @@ -113,7 +108,7 @@ export class ExplorationRecommendationsService {
'/explorehandler/recommendations/' +
ExplorationRecommendationsService.explorationId, {
params: recommendationsUrlParams
}).toPromise().then((data: ISummaries) => {
}).toPromise().then((data: IExplorationSummaries) => {
successCallback(data.summaries);
});
}
Expand Down
3 changes: 0 additions & 3 deletions scripts/linters/excluded_any_type_files.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
"core/templates/domain/classifier/AnswerClassificationResultObjectFactorySpec.ts",
"core/templates/domain/classifier/ClassifierObjectFactory.ts",
"core/templates/domain/classroom/classroom-backend-api.service.ts",
"core/templates/domain/collection/CollectionNodeObjectFactory.ts",
"core/templates/domain/collection/CollectionObjectFactory.ts",
"core/templates/domain/collection/CollectionPlaythroughObjectFactory.ts",
"core/templates/domain/collection/CollectionRightsObjectFactory.ts",
"core/templates/domain/collection/collection-rights-backend-api.service.ts",
Expand All @@ -20,7 +18,6 @@
"core/templates/domain/collection/editable-collection-backend-api.service.ts",
"core/templates/domain/collection/read-only-collection-backend-api.service.ts",
"core/templates/domain/collection/search-explorations-backend-api.service.ts",
"core/templates/domain/editor/undo_redo/ChangeObjectFactory.ts",
"core/templates/domain/exploration/AnswerGroupObjectFactory.ts",
"core/templates/domain/exploration/AnswerStatsObjectFactory.ts",
"core/templates/domain/exploration/ExplorationDraftObjectFactory.ts",
Expand Down