Skip to content

Commit

Permalink
fix(all): FAQ Menu fixed.
Browse files Browse the repository at this point in the history
  • Loading branch information
rodrigorodriguez committed Feb 28, 2021
1 parent f5ee845 commit 7c93328
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 9 deletions.
31 changes: 29 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,33 @@ Your pull request should:
* Have clear commit messages
* e.g. "Refactor feature", "Fix issue", "Add tests for issue"

## Running (and adding) the Tests
## You need to be able to run your system

from: http://catern.com/run.html

When developing a system, it is important to be able to run the system in its entirety.
"Run the unit tests" doesn't count. The complexity of your system is in the interactions between the units.

"Run an individual service against mocks" doesn't count. A mock will rarely behave identically to the real dependency, and the behavior of the individual service will be unrealistic. You need to run the actual system.

"Run an individual service in a shared stateful development environment running all the other services" doesn't count. A shared development environment will be unreliable as it diverges more and more from the real system.

"Run most services in a mostly-isolated development environment, calling out to a few hard-to-run external services" doesn't count. Those few external services on the edge of the mostly-isolated development environment are often the most crucial ones; without the ability to run modified versions of them, your development process is crippled. Furthermore, being dependent on external services greatly complicates where and how you can run the system; it's much harder to, for example, run tests with the system on every commit if that will access external services.

"Run all the services that make up the system in an isolated development environment" counts; it's the bare minimum requirement. Bonus points if this can be done completely on localhost, without using an off-host cluster deployment system.

Without the ability to actually run the entire system in this way while developing, many evil practices will tend to become common.

Testing is harder and far less representative, and therefore many issues can only be found when changes are deployed to production.
In turn, production deployment will cause issues more often, and so deployment will be more slow and less frequent.
Deploying the system to new environments is more difficult, since the developers aren't able to actually run the system. Existing practices in production will be cargo-culted and copied around indefinitely, even when they are unnecessary or actively harmful.
Exploratory usage of the system is very difficult, so it will be harder to consider using the system for purposes outside what it was originally developed for, and new use cases will become rare.
Downstream clients who depend on the system will also suffer all these issues, since without the ability to run the upstream system in development, they can't run their own entire system, which is a superset of the upstream system.
Running the entire system during development is the first step to preventing these issues. Further steps include writing automated tests for the system (which can be run repeatedly during development), and using, as much as possible, the same code to run the system in development and in production.
Developers of large or legacy systems that cannot already be run in their entirety during development often believe that it is impractical to run the entire system during development. They'll talk about the many dependencies of their system, how it requires careful configuration of a large number of hosts, or how it's too complex to get reliable behavior.

In my experience, they're always wrong. These systems can be run locally during development with a relatively small investment of effort. Typically, these systems are just ultimately not as complicated as people think they are; once the system's dependencies are actually known and understood rather than being cargo-culted or assumed, running the system, and all its dependencies, is straightforward.

Being able to run your entire system during development is just about the most basic requirement for a software project. It's not, on its own, sufficient for your development practices to be high quality; but if you can't do this, then you're not even in the running.


*Coming soon*
7 changes: 5 additions & 2 deletions packages/basic.gblib/services/SystemKeywords.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ export class SystemKeywords {
*
*/
public async get(file: string, address: string): Promise<any> {
GBLog.info(`BASIC: GET '${address}' in '${file}'.`);
let [baseUrl, client] = await this.internalGetDriveClient();
const botId = this.min.instance.botId;
const path = `/${botId}.gbai/${botId}.gbdata`;
Expand Down Expand Up @@ -288,7 +289,8 @@ export class SystemKeywords {
* loop
*
*/
public async find(file: string, ...args): Promise<any> {
public async find(file: string, ...args ): Promise<any> {
GBLog.info(`BASIC: FIND running on ${file}...`);
let [baseUrl, client] = await this.internalGetDriveClient();
const botId = this.min.instance.botId;
const path = `/${botId}.gbai/${botId}.gbdata`;
Expand Down Expand Up @@ -441,7 +443,7 @@ export class SystemKeywords {
*
*/
public async copyFile(src, dest) {
GBLog.info(`BASIC: COPY '${src}' to '${dest}'`);
GBLog.info(`BASIC: BEGINING COPY '${src}' to '${dest}'`);
let [baseUrl, client] = await this.internalGetDriveClient();
const botId = this.min.instance.botId;

Expand Down Expand Up @@ -495,6 +497,7 @@ export class SystemKeywords {
}
throw error;
}
GBLog.info(`BASIC: FINISHED COPY '${src}' to '${dest}'`);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/core.gbapp/services/GBMinService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ export class GBMinService {
secret: instance.webchatKey
});
const service = new KBService(min.core.sequelize);
const data = await service.getFaqBySubjectArray('faq', undefined);
const data = await service.getFaqBySubjectArray(instance.instanceId, 'faq', undefined);
await min.conversationalService.sendEvent(min, step, 'play', {
playerType: 'bullet',
data: data.slice(0, 10)
Expand Down
2 changes: 1 addition & 1 deletion packages/kb.gbapp/dialogs/FaqDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class FaqDialog extends IGBDialog {

min.dialogs.add(new WaterfallDialog('/faq', [
async step => {
const data = await service.getFaqBySubjectArray('faq', undefined);
const data = await service.getFaqBySubjectArray(min.instance.instanceId, 'faq', undefined);
const locale = step.context.activity.locale;
if (data !== undefined) {
await min.conversationalService.sendEvent(min, step, 'play', {
Expand Down
3 changes: 2 additions & 1 deletion packages/kb.gbapp/dialogs/MenuDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export class MenuDialog extends IGBDialog {

// Whenever a subject is selected, shows a faq about it.
if (user.subjects.length > 0) {
const list = await service.getFaqBySubjectArray('menu', user.subjects);
const list = await service.getFaqBySubjectArray(min.instance.instanceId,
'menu', user.subjects);
await min.conversationalService.sendEvent(min, step, 'play', {
playerType: 'bullet',
data: list.slice(0, 10)
Expand Down
6 changes: 4 additions & 2 deletions packages/kb.gbapp/services/KBService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ export class KBService implements IGBKBService {
});
}

public async getFaqBySubjectArray(from: string, subjects: any): Promise<GuaribasQuestion[]> {
public async getFaqBySubjectArray(instanceId: number, from: string, subjects: any): Promise<GuaribasQuestion[]> {
if (subjects) {
const where = {
from: from,
Expand All @@ -333,7 +333,9 @@ export class KBService implements IGBKBService {
// tslint:disable-next-line: no-null-keyword
subject3: null,
// tslint:disable-next-line: no-null-keyword
subject4: null
subject4: null,
// tslint:disable-next-line: no-null-keyword
instanceId: instanceId
};

if (subjects[0] && subjects[0].internalId) {
Expand Down

0 comments on commit 7c93328

Please sign in to comment.