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

Webhooks change for prompts used in filling other tables/columns #69

Closed
2 tasks done
Satendra-SR opened this issue Mar 31, 2021 · 9 comments
Closed
2 tasks done
Assignees
Labels
enhancement New feature or request priority: high This issue or pull request is on high priority Sprint: 1 (May) 3 - 14 May

Comments

@Satendra-SR
Copy link
Member

Satendra-SR commented Mar 31, 2021

Is your feature request related to a problem? Please describe.
The system is capturing ivr prompt response in ivr_prompt_response table. But for populating other table column, we need to make changes for each data set. We need to comeup with a plan to do it efficiently.

Describe the solution you'd like
We can divide the item into two sub items:

  • Research and come up with a plan to make it efficient and how we can reduce this cycle.
  • Execution, need to make those changes in the system

Dev Notes:
We can consider making changes in prompt naming conventions so that we can easily extract the info from prompt itself.

Describe alternatives you've considered
Check if we can pass additional info from Rapid Pro itself.

Additional context
None

@Satendra-SR Satendra-SR added the enhancement New feature or request label Mar 31, 2021
@Satendra-SR Satendra-SR self-assigned this Mar 31, 2021
@Satendra-SR Satendra-SR added the priority: high This issue or pull request is on high priority label Mar 31, 2021
@Satendra-SR
Copy link
Member Author

Satendra-SR commented Apr 5, 2021

There are multiple ways we can handle this, sharing the possible ways below:

  1. Add table + column name to prompts itself: In this case, the system at the backend will check if the table name exists in the prompt name, make the changes
    • Advantages:
      • It should be easy to implement.
      • Minimal code changes required when adding a new prompt.
    • Shortcomings:
      • Need to re-look into naming conventions we discussed earlier. It can take longer as there is an existing nomenclature and it can impact others as well.
  2. Define mapping at codebase: In this case, we will add a mapping at a codebase, and whenever
    • Advantages:
      • It should be easy to implement.
    • Shortcomings:
      • Changes will be required at codebase when adding a new prompt.
  3. Define mapping at the database level:
    1. If 1-1 mapping exists (1 prompt will be responsible for populating a single table's column), add new columns to the same table.
    2. If a 1-n mapping exists(1 prompt response will be responsible to populate multiple tables/columns) add a new table for mapping prompts and other tables mapping.
    • Advantages:
      • No code changes will be required at the code base when adding a new prompt
    • Shortcomings:
      • The pre-seeding data will be required each time when adding a new prompt (We are anyway adding these data while adding prompt details)

All three options are doable and will work fine. I would recommend going with the third option as it will behave a clean implementation, we don't have to make changes for adding a new prompt and we will have data available (in the database) if in case we need it.

@prtkdost
Copy link

prtkdost commented Apr 5, 2021

I also feel 3rd option is good.

@Satendra-SR
Copy link
Member Author

We are going with the third option. Based on our use case, one-one mapping is what we need right now. This means, one prompt will be responsible for populating any single column of a particular table.

@Satendra-SR
Copy link
Member Author

We can use phone number as identifier.

Satendra-SR added a commit that referenced this issue Apr 29, 2021
…pt-used-in-filling-other-tables

#69 Changes for prompt used in filling other tables
@Satendra-SR
Copy link
Member Author

Satendra-SR commented May 4, 2021

Test steps:

  • Test all four condition:
    • Morning
    • Afternoon
    • Evening
    • Other/Default
  • Overriden
  • Test whether multiple columns in multiple tables are getting filled.

@Satendra-SR
Copy link
Member Author

Satendra-SR commented May 4, 2021

Need to test the multiple system phone, it is failing for now

Update

Created a new story #92 for this.

@Satendra-SR
Copy link
Member Author

Satendra-SR commented May 4, 2021

Need to update based on registration id. Create a new issue.

Update

Created a new story #93 for this.

@Satendra-SR
Copy link
Member Author

Testing this item on staging, moving to accepted.

@Satendra-SR
Copy link
Member Author

@prtkdost Deployed in on production. I've seeded the mapping table for the preferred time slot as well. Closing this item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: high This issue or pull request is on high priority Sprint: 1 (May) 3 - 14 May
Projects
None yet
Development

No branches or pull requests

2 participants