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

cc: ensure additional responses are saved properly #1676

Closed

Conversation

jo3-l
Copy link
Contributor

@jo3-l jo3-l commented Jun 20, 2024

Currently, if one uses the + button to create a new response for a custom command, then save, only the first response is persisted: that is, additional responses are lost and not saved.

This bug is a regression from the previous version. I bisected it to the CodeMirror change, #1656. The issue is that we are not saving changes to the underlying textarea for newly added CodeMirror instances, so the POSTed form always contains empty responses (save the first one.) This PR therefore patches setupCodeMirrorInstance to not merely initialize the CodeMirror instance but also attach a change event listener.

@ashishjh-bst perhaps we could consider a cherry-pick to the current release branch, given that this bug does result in lost data if one attempts to add an additional custom command response.

savage4618 and others added 7 commits June 18, 2024 12:00
changed wording to be clearer about which tiers of Patreon grant premium slots.
…bs-gg#1672)

* dcmd: add manual response marker

...designed for commands that send their responses manually as opposed to through dcmd, such as the poll command.

* poll,wyr: indicate to dcmd that responses are sent manually

This commit (combined with the previous) allows poll and wyr responses to be autodeleted after a delay via command overrides. Previously, the command system had no knowledge of responses to these commands (which are sent manually since reactions need to be added), so autodeletion did not work.
…otlabs-gg#1671)

Currently, commands with paginated responses do not obey the 'delete response after delay' option set using command overrides. The reason is that dcmd (and in turn the YAGPDB command system) does not know about paginated messages -- which are sent manually as opposed to using dcmd's response mechanism -- and so naturally does not delete them.

This commit therefore teaches dcmd about paginated messages by introducing a new type, PaginatedResponse, which implements dcmd.Response. When returned from a command run function, dcmd will then automatically call PaginatedResponse.Send to create and return the paginated message, which interopates with the existing response deletion machinery.
…g#1655)

* all: migrate executed command logging away from gorm to sqlboiler (botlabs-gg#1654)

* common: add schemas for executed command logs

* common: configure and run sqlboiler for executed command log model

* all: migrate executed command logging to sqlboiler

* reminders: migrate away from gorm to sqlboiler (botlabs-gg#1652)

* reminders: add database schemas

* reminders: set up and run sqlboiler

* reminders: migrate to sqlboiler

While at it, lightly refactor and simplify the code as applicable. The most substantial (non-database) refactor is renaming and moving strReminders to DisplayReminders.

To aid in the refactor, also add discordgo.ParseID as a shortcut for strconv.ParseInt(...). This operation is often needed to convert string IDs in database to int64's and having a shorter name helps.

* reminders: auto-delete old entries with null guild_id as necessary

* soundboard: remove stray references to gorm (botlabs-gg#1651)

The soundboard module used to use gorm, but was migrated to sqlboiler some years ago in commit 628ea9a. There are still two (erroneous) references to gorm remaining which were not caught since this section of code ignores errors. This commit changes these to use sqlboiler as well.

* schemamigration: skip executing ALTER COLUMN SET NOT NULL queries when possible (botlabs-gg#1650)

* schemamigration: skip executing ALTER COLUMN SET NOT NULL queries when possible

* Use quantifier * not + in regex for consistency

It really should be +, but the current code works and hey -- when in Rome, do as the Romans.

* youtube: migrate away from gorm to sqlboiler (botlabs-gg#1660)

* youtube: add database schemas

* youtube: set up and run sqlboiler

* youtube: migrate to sqlboiler

* notifications: migrate away from gorm to sqlboiler (botlabs-gg#1659)

* notifications: decouple from configstore

* notifications: add database schemas

* notifications: set up and run sqlboiler

* notifications: migrate to sqlboiler

* moderation: migrate away from gorm to sqlboiler (botlabs-gg#1658)

* moderation: decouple from configstore

* moderation: add database schemas

* moderation: set up and run sqlboiler

* moderation: migrate to sqlboiler

* web: support null.Int64 in forms and validator

We currently use gorilla/schema to parse HTML form values into structs -- this allows us to specify, e,g., `<input ... name=SomeField>` and have `payload.SomeField` set correctly in Go handlers. This approach mostly works nicely, with the major exception of null.Int64 (formerly sql.NullInt64 with gorm) fields, which show up in the moderation plugin, e.g., Config.DefaultMuteDuration.

Particularly, since gorilla/schema has no native support for null.Int64, we have instead instructed gorilla/schema to directly set the Int64 value using <input ... name=DefaultMuteDuration.Int64> and then manually set DefaultMuteDuration.Valid=true in the web handler, without which the changes are not saved to database. This approach is, for obvious reasons, rather brittle. In this commit we therefore register a custom gorilla/schema decoder for null.Int64, obviating the need for both pointing to the Int64 field and manually setting Valid=true. While at it, we also fix the validator so that it checks null.Int64 fields properly.

* web: support validating types.Int64Array

gorm uses pq.Int64Array but sqlboiler uses types.Int64Array, so we need also support the latter now.

* common/configstore: remove package (botlabs-gg#1668)

* all: remove last traces of gorm (botlabs-gg#1670)

* common: connect to postgres via database/sql

...instead of through gorm.

* common: remove gorm log hook

* common: remove gorm SmallModel

* deps: run go mod tidy

---------

Co-authored-by: Joe L <[email protected]>
@ashishjh-bst ashishjh-bst changed the base branch from dev to hotfixes June 20, 2024 04:59
@ashishjh-bst
Copy link
Contributor

Closing this as I have cherry picked the commit into another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants