Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 86 additions & 0 deletions REVIEW_AND_PROPOSAL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Review and Proposal: BotPlatform Refactoring

## 1. Review of Current State

The `BotPlatform` repository is currently tightly coupled with the `ScriptureBot` application. This coupling prevents `BotPlatform` from being a truly "democratized" and generic platform for other chatbots.

### Key Issues Identified:

1. **Data Structure Coupling (`pkg/def/class.go`)**:
* **`UserData`**: Contains `datastore:""` tags. These are specific to Google Cloud Datastore and the schema used by `ScriptureBot`. A generic platform should be storage-agnostic.
* **`UserData`**: Contains `Action` and `Config` fields. These are application-level state tracking fields specific to ScriptureBot's state machine logic, not properties of a Platform User.
* **`SessionData`**: Contains `ResourcePath string`. This is a `ScriptureBot`-specific configuration used to locate local resources. Generic session data should not enforce specific configuration fields.
* **UI Constraints**: The generic `ResponseOptions` struct forces a 1-column layout (via hardcoded constants in the Telegram implementation), limiting flexibility for other bots.

2. **Platform Implementation (`pkg/platform/telegram.go`)**:
* The `Translate` method populates `env.User` directly into the struct. While functional, it needs to ensure generic extensibility points (like `Props`) are initialized.

3. **ScriptureBot Usage**:
* `ScriptureBot` relies on `BotPlatform`'s `UserData` for its database operations (`utils.RegisterUser`, `utils.PushUser`) and state tracking (`Action`).
* `ScriptureBot` uses `SessionData.ResourcePath` to pass configuration.

## 2. Refactoring Proposal for BotPlatform

The goal is to remove all `ScriptureBot`-specific artifacts from `BotPlatform` while providing extension points so `ScriptureBot` (and other bots) can still function effectively.

### Proposed Changes:

1. **Clean `UserData`**:
* Remove all `datastore` tags from the `UserData` struct.
* Remove `Action` and `Config` fields. `UserData` should only contain fields relevant to the chat platform identity (Id, Username, Firstname, Lastname, Type).

2. **Generalize `SessionData`**:
* Remove `ResourcePath` from `SessionData`.
* Add a generic `Props map[string]interface{}`. This allows applications to attach arbitrary data (like `ResourcePath` or other context) to the session.
* **Crucial Implementation Detail**: Platform implementations (e.g., `Translate` in `telegram.go`) *must* initialize this map (`make(map[string]interface{})`) to prevent runtime panics for consumers.

3. **Enhance UI Flexibility**:
* Add `ColWidth int` to `ResponseOptions`.
* Update platform logic to use this value for button layout, defaulting to the standard (1 column) if not set.

## 3. Adaptation Plan for ScriptureBot

Since `BotPlatform` will be modifying its public API, `ScriptureBot` must be updated.

### Required Changes in ScriptureBot:

1. **Define Local User Model**:
* Create a `User` struct in `ScriptureBot` (e.g., in `pkg/models/user.go`) that includes:
* The basic fields (Firstname, etc.)
* The `datastore` tags.
* **The State Fields**: `Action` and `Config`.
* Example:
```go
type User struct {
Firstname string `datastore:""`
Action string `datastore:""`
Config string `datastore:""`
// ... other fields
}
```

2. **Map Data**:
* In `TelegramHandler`, map `platform.UserData` (identity) to `ScriptureBot.User` (identity + state).
* Load `Action` and `Config` from the database (via `utils.RegisterUser`), not from the platform session.

3. **Handle ResourcePath**:
* Populate `env.Props["ResourcePath"]` in the handler and read it from there in command processors.

## 4. Migration Impact Analysis

### Will this affect existing users?
**No, the data for existing users will remain intact.**

* **Data Compatibility**: The removal of fields (`Action`, `Config`) from the *library struct* does not delete columns in the *database*. Since `ScriptureBot` will define a local struct that *includes* these fields before writing back to the DB, the data is preserved.
* **Datastore Tags**: Removing tags is safe as the Go Datastore client defaults to field names, which matches the previous behavior.

### Do we need a migration task?
**Yes, a *Code Migration* task is required.**

`ScriptureBot` **will fail to compile** or **lose state functionality** without code changes because `UserData` will no longer have `Action` or `Config`.

* **Task**: Implement the "Define Local User Model" step. This is critical to preserve the bot's ability to remember user state (e.g., "waiting for search term").

## 5. Conclusion

This refactoring strictly separates "Platform Identity" from "Application State" and "Storage". `BotPlatform` handles the delivery of messages, while `ScriptureBot` owns the user's state and data persistence.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
cloud.google.com/go/datastore v1.20.0
cloud.google.com/go/secretmanager v1.16.0
github.com/joho/godotenv v1.5.1
github.com/julwrites/BotPlatform v0.0.0-20251128175347-656700b2e4d4
github.com/julwrites/BotPlatform v0.0.0-20251211011140-ceb9fd2844a7
golang.org/x/net v0.43.0
google.golang.org/api v0.247.0
gopkg.in/yaml.v2 v2.4.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
github.com/julwrites/BotPlatform v0.0.0-20251128175347-656700b2e4d4 h1:NeEPkJt4VXvyb/zxggO9zFs9tAJ07iiht1lAkUL/ZNs=
github.com/julwrites/BotPlatform v0.0.0-20251128175347-656700b2e4d4/go.mod h1:PZT+yPLr4MrricOGOhXwiJCurNcGj36fD1jZOwMiuIk=
github.com/julwrites/BotPlatform v0.0.0-20251211011140-ceb9fd2844a7 h1:v2a+Vzsy9v1+qrgpDA42t/ORWUoq9W/eMIhy5SMbAIc=
github.com/julwrites/BotPlatform v0.0.0-20251211011140-ceb9fd2844a7/go.mod h1:PZT+yPLr4MrricOGOhXwiJCurNcGj36fD1jZOwMiuIk=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
Expand Down
2 changes: 1 addition & 1 deletion pkg/app/ask.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func GetBibleAsk(env def.SessionData) def.SessionData {

func GetBibleAskWithContext(env def.SessionData, contextVerses []string) def.SessionData {
if len(env.Msg.Message) > 0 {
config := utils.DeserializeUserConfig(env.User.Config)
config := utils.DeserializeUserConfig(utils.GetUserConfig(env))

req := QueryRequest{
Query: QueryObject{
Expand Down
8 changes: 4 additions & 4 deletions pkg/app/ask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestGetBibleAsk(t *testing.T) {
env.Msg.Message = "Who is God?"
env.User.Id = "12345"
conf := utils.UserConfig{Version: "NIV"}
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))

// Set dummy API config
SetAPIConfigOverride("https://mock", "key")
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestGetBibleAsk(t *testing.T) {
var env def.SessionData
env.Msg.Message = "Explain this"
conf := utils.UserConfig{Version: "NIV"}
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))
contextVerses := []string{"John 3:16", "Genesis 1:1"}

// Set dummy API config
Expand Down Expand Up @@ -85,7 +85,7 @@ func TestGetBibleAsk(t *testing.T) {
env.User.Id = "user_id"
env.Msg.Message = "Question"
conf := utils.UserConfig{Version: "NIV"}
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))

env = GetBibleAsk(env)

Expand All @@ -106,7 +106,7 @@ func TestGetBibleAsk(t *testing.T) {
env.User.Id = "admin_id"
env.Msg.Message = "Question"
conf := utils.UserConfig{Version: "NIV"}
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))

env = GetBibleAsk(env)

Expand Down
3 changes: 2 additions & 1 deletion pkg/app/close.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"math/rand"

"github.com/julwrites/BotPlatform/pkg/def"
"github.com/julwrites/ScriptureBot/pkg/utils"
)

var CLOSEMSGS = []string{
Expand All @@ -17,7 +18,7 @@ var CLOSEMSGS = []string{

func CloseAction(env def.SessionData) def.SessionData {
env.Res.Affordances.Remove = true
env.User.Action = ""
env = utils.SetUserAction(env, "")

fmtMessage := CLOSEMSGS[rand.Intn(len(CLOSEMSGS))]

Expand Down
13 changes: 8 additions & 5 deletions pkg/app/database_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,18 @@ func TestUserDatabaseIntegration(t *testing.T) {

// Create/Update user
// This exercises the connection to Datastore/Firestore
updatedUser := utils.RegisterUser(user, projectID)
localUser := utils.RegisterUser(user, projectID)

if updatedUser.Id != dummyID {
t.Errorf("Expected user ID %s, got %s", dummyID, updatedUser.Id)
if localUser.Id != dummyID {
t.Errorf("Expected user ID %s, got %s", dummyID, localUser.Id)
}

// Verify update capability
updatedUser.Action = "testing"
finalUser := utils.RegisterUser(updatedUser, projectID)
localUser.Action = "testing"
utils.PushUser(localUser, projectID)

// Retrieve again
finalUser := utils.RegisterUser(user, projectID)

if finalUser.Action != "testing" {
t.Errorf("Expected user Action 'testing', got '%s'", finalUser.Action)
Expand Down
7 changes: 4 additions & 3 deletions pkg/app/devo.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/julwrites/BotPlatform/pkg/platform"

"github.com/julwrites/BotPlatform/pkg/def"
"github.com/julwrites/ScriptureBot/pkg/utils"
)

const (
Expand Down Expand Up @@ -168,7 +169,7 @@ func GetDevotionalData(env def.SessionData, devo string) def.ResponseData {
}

func GetDevo(env def.SessionData, bot platform.Platform) def.SessionData {
switch env.User.Action {
switch utils.GetUserAction(env) {
case CMD_DEVO:
log.Printf("Detected existing action /devo")

Expand All @@ -185,7 +186,7 @@ func GetDevo(env def.SessionData, bot platform.Platform) def.SessionData {
// Retrieve devotional
env.Res = GetDevotionalData(env, devo)

env.User.Action = ""
env = utils.SetUserAction(env, "")
} else {
log.Printf("AcronymizeDevo failed %v", err)
env.Res.Message = "I didn't recognize that devo, please try again"
Expand All @@ -202,7 +203,7 @@ func GetDevo(env def.SessionData, bot platform.Platform) def.SessionData {

env.Res.Affordances.Options = options

env.User.Action = CMD_DEVO
env = utils.SetUserAction(env, CMD_DEVO)

env.Res.Message = "Choose a Devotional to read!"
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/app/devo_brp.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func GetNavigators5xDatabase(dataPath string) DailyChapterBRP {
func GetDiscipleshipJournalReferences(env def.SessionData) []def.Option {
var options []def.Option

djBRP := GetDiscipleshipJournalDatabase(env.ResourcePath)
djBRP := GetDiscipleshipJournalDatabase(utils.GetResourcePath(env))

// We will read the entry using the date, format: Year, Month, Day

Expand All @@ -147,7 +147,7 @@ func GetDiscipleshipJournalReferences(env def.SessionData) []def.Option {
return options
}
func GetDailyNewTestamentReadingReferences(env def.SessionData) string {
DNTBRP := GetDailyNewTestamentDatabase(env.ResourcePath)
DNTBRP := GetDailyNewTestamentDatabase(utils.GetResourcePath(env))

// We will read the entry using the date, format: Year, Month, Day
baseline := time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC)
Expand All @@ -161,7 +161,7 @@ func GetDailyNewTestamentReadingReferences(env def.SessionData) string {
func GetNavigators5xRestDayPrompt(env def.SessionData) (string, []def.Option) {
var options []def.Option

N5XBRP := GetNavigators5xDatabase(env.ResourcePath)
N5XBRP := GetNavigators5xDatabase(utils.GetResourcePath(env))

// We will read the entry using the date, format: Year, Month, Day
dateIndex := time.Now().YearDay()
Expand Down Expand Up @@ -196,7 +196,7 @@ Here are this week's passages!
}

func GetNavigators5xReferences(env def.SessionData) string {
N5XBRP := GetNavigators5xDatabase(env.ResourcePath)
N5XBRP := GetNavigators5xDatabase(utils.GetResourcePath(env))

// We will read the entry using the date, format: Year, Month, Day
day := time.Now().YearDay()
Expand Down
14 changes: 7 additions & 7 deletions pkg/app/devo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/julwrites/BotPlatform/pkg/def"
"github.com/julwrites/ScriptureBot/pkg/utils"
)

func TestGetMCheyneHtml(t *testing.T) {
Expand Down Expand Up @@ -33,13 +34,12 @@ func TestGetDiscipleshipJournalDatabase(t *testing.T) {

func TestGetDiscipleshipJournalReferences(t *testing.T) {
var env def.SessionData

env.ResourcePath = "../../resource"
env.Props = map[string]interface{}{"ResourcePath": "../../resource"}

options := GetDiscipleshipJournalReferences(env)

if len(options) == 0 {
djBRP := GetDiscipleshipJournalDatabase(env.ResourcePath)
djBRP := GetDiscipleshipJournalDatabase(utils.GetResourcePath(env))

length := len(djBRP.BibleReadingPlan) / 12

Expand Down Expand Up @@ -82,7 +82,7 @@ func TestGetDevotionalData(t *testing.T) {
ResetAPIConfigCache()

var env def.SessionData
env.ResourcePath = "../../resource"
env.Props = map[string]interface{}{"ResourcePath": "../../resource"}
env.Res = GetDevotionalData(env, "DTMSV")

if len(env.Res.Message) == 0 {
Expand All @@ -98,7 +98,7 @@ func TestGetDevo(t *testing.T) {
ResetAPIConfigCache()

var env def.SessionData
env.User.Action = ""
env = utils.SetUserAction(env, "")
env.Msg.Message = CMD_DEVO

env = GetDevo(env, &MockBot{})
Expand All @@ -119,9 +119,9 @@ func TestGetDevo(t *testing.T) {
ResetAPIConfigCache()

var env def.SessionData
env.User.Action = CMD_DEVO
env = utils.SetUserAction(env, CMD_DEVO)
env.Msg.Message = devoName
env.ResourcePath = "../../resource"
env.Props = map[string]interface{}{"ResourcePath": "../../resource"}

env = GetDevo(env, &MockBot{})

Expand Down
5 changes: 3 additions & 2 deletions pkg/app/natural_language_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/julwrites/BotPlatform/pkg/def"
"github.com/julwrites/ScriptureBot/pkg/utils"
)

func TestProcessNaturalLanguage(t *testing.T) {
Expand Down Expand Up @@ -102,7 +103,7 @@ func TestProcessNaturalLanguage(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
env := def.SessionData{}
env.Msg.Message = tt.message
env.User.Config = `{"version":"NIV"}`
env = utils.SetUserConfig(env, `{"version":"NIV"}`)

res := ProcessNaturalLanguage(env)

Expand All @@ -111,4 +112,4 @@ func TestProcessNaturalLanguage(t *testing.T) {
}
})
}
}
}
4 changes: 2 additions & 2 deletions pkg/app/passage.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func ParsePassageFromHtml(ref string, rawHtml string, version string) string {
}

func GetBiblePassageFallback(env def.SessionData) def.SessionData {
config := utils.DeserializeUserConfig(env.User.Config)
config := utils.DeserializeUserConfig(utils.GetUserConfig(env))

doc := GetPassageHTML(env.Msg.Message, config.Version)
ref := GetReference(doc)
Expand Down Expand Up @@ -194,7 +194,7 @@ func GetBiblePassage(env def.SessionData) def.SessionData {
env.Msg.Message = ref
}

config := utils.DeserializeUserConfig(env.User.Config)
config := utils.DeserializeUserConfig(utils.GetUserConfig(env))

// If indeed a reference, attempt to query
if len(ref) > 0 {
Expand Down
16 changes: 2 additions & 14 deletions pkg/app/passage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,9 @@ import (
"github.com/julwrites/ScriptureBot/pkg/utils"
)

func TestGetBiblePassageHtml(t *testing.T) {
doc := GetPassageHTML("gen 8", "NIV")

if doc == nil {
t.Errorf("Could not retrieve bible passage")
}
}

func TestGetReference(t *testing.T) {
doc := GetPassageHTML("gen 1", "NIV")

if doc == nil {
t.Fatalf("Could not retrieve Bible passage for testing")
}

ref := GetReference(doc)

if ref != "Genesis 1" {
Expand Down Expand Up @@ -59,7 +47,7 @@ func TestGetBiblePassage(t *testing.T) {
env.Msg.Message = "gen 1"
var conf utils.UserConfig
conf.Version = "NIV"
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))

// Set dummy API config to pass internal checks
SetAPIConfigOverride("https://mock", "key")
Expand Down Expand Up @@ -89,7 +77,7 @@ func TestGetBiblePassage(t *testing.T) {
env.Msg.Message = "gen 1"
var conf utils.UserConfig
conf.Version = "NIV"
env.User.Config = utils.SerializeUserConfig(conf)
env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf))
env = GetBiblePassage(env)

if len(env.Res.Message) < 10 {
Expand Down
Loading
Loading