Skip to content
This repository has been archived by the owner on Aug 28, 2020. It is now read-only.

SettingsGateway: The Final Rewrite #875

Closed
wants to merge 4 commits into from

Conversation

kyranet
Copy link
Contributor

@kyranet kyranet commented Nov 11, 2019

Description of the PR

Prepares klasa for @klasa/settings-gateway.

Changes Proposed in this Pull Request (List new items in CHANGELOG.MD)

Added

  • Added Unit Testing.
  • Added examples for a large set of methods.
  • Added bare null as an option to reset a key in SettingsFolder#{update}.
  • Added Serializer#{validate,resolve} to allow further control on how SettingsGateway handles the data.
  • Added context to the settingsUpdate and settingsCreate events, they contain the raw changes, guild, language, etc.
  • Added extraContext to the SettingsFolderResetOptions type, this value is pased in all places (Serializer#validate, SchemaEntry#filter, settingsUpdate and settingsCreate events, and more).

Changed

  • Tweaked Serializer#stringify's second argument from KlasaMessage to Guild | null.
  • Tweaked Serializer#deserialize's arguments to (SerializableValue, SerializerUpdateContext).
  • Renamed SchemaEntry#{min,max} to SchemaEntry#{minimum,maximum}.
  • Tweaked SettingsFolder's value type to be more accurate.
  • Tweaked SettingsFolderUpdateOptions's option to produce a TypeScript compiler error when arrayAction is set to 'overwrite' and arrayIndex is defined.
  • Tweaked SettingsFolder#client to throw an error when it's uninitialized.
  • Tweaked SettingsFolder#{reset,update} to return a much more useful struct.
  • When specifying arrayIndex in SettingsFolder#update and arrayAction is defined as add, all entries will be inserted at given index.
  • When specifying arrayIndex in SettingsFolder#update and arrayAction is defined as remove, as many entries as given will be removed from given index.
  • When specifying arrayIndex in SettingsFolder#update and arrayAction is not defined or defined as auto, all entries will replace the existing ones.

Removed

  • Removed throwOnError option in SettingsFolder#{reset,update}, they will now always throw when they encounter an error.

Fixed

  • Resolved bug where Schema#get would throw an error if a path did not exist.
  • Resolved bug where Schema#{add,remove} was still callable even after being initialized.
  • Resolved bug where SettingsFolder#get would throw an error if a path did not exist.
  • Resolved type bug in SettingsFolder#pluck.
  • Resolved bug in SettingsFolder#resolve not resolving into objects when specifying a folder path.
  • Resolved bug in SettingsFolder#reset where database conditions were not handled correctly.
  • Resolved bug in SettingsFolder#update where options would sometimes type error.
  • Resolved bug in SettingsFolder's patch function not allowing non-literal objects to be used.
  • Resolved bug in SettingsFolder#{reset,update} patching after emit.
  • Fixed the types from the Provider and SQLProvider classes.

Semver Classification

  • This PR only includes documentation or non-code changes.
  • This PR fixes a bug and does not change the (intended) framework interface.
  • This PR adds methods or properties to the framework interface.
  • This PR removes or renames methods or properties in the framework interface.

@kyranet kyranet added Type: Enhancement Issues and PRs related to feature enhancement. Status: Help Wanted Issues that need assistance from volunteers or PRs that need help to proceed. Meta: Refactor Issues and PRs related to refactors. SEM: Major PRs that contain breaking changes and should be released in the next major version. Meta: BugFix PRs that fix bugs or issues. Meta: Dependencies Issues and PRs related to dependencies. Status: Needs Testing PRs that need testing from the author or volunteers. Meta: Cleanup Issues and PRs related to code cleanup. Mod: SettingsGateway Issues and PRs related to SettingsGateway. Meta: Typings Issues and PRs related to typings. labels Nov 11, 2019
@bdistin
Copy link
Contributor

bdistin commented Nov 11, 2019

img

@lgtm-com

This comment has been minimized.

@kyranet kyranet marked this pull request as ready for review January 2, 2020 18:12
@kyranet kyranet removed the Status: Help Wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jan 2, 2020
@kyranet kyranet force-pushed the wipe-settings branch 7 times, most recently from 4bf89b9 to 8772410 Compare January 2, 2020 23:28
src/commands/Admin/conf.js Show resolved Hide resolved
Copy link
Member

@PyroTechniac PyroTechniac left a comment

Choose a reason for hiding this comment

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

Looked through it a second time and found a few things

src/commands/Admin/conf.js Outdated Show resolved Hide resolved
src/commands/General/User Settings/userconf.js Outdated Show resolved Hide resolved
Co-Authored-By: Gryffon Bellish <owenbellish@gmail.com>
src/commands/Admin/conf.js Outdated Show resolved Hide resolved
src/commands/General/User Settings/userconf.js Outdated Show resolved Hide resolved
Co-Authored-By: Gryffon Bellish <owenbellish@gmail.com>
Copy link
Member

@PyroTechniac PyroTechniac left a comment

Choose a reason for hiding this comment

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

LGTM

@kyranet
Copy link
Contributor Author

kyranet commented Feb 1, 2020

Superseded by #916 to move the base branch into this repository.

@kyranet kyranet closed this Feb 1, 2020
@kyranet kyranet deleted the wipe-settings branch February 1, 2020 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Meta: BugFix PRs that fix bugs or issues. Meta: Cleanup Issues and PRs related to code cleanup. Meta: Dependencies Issues and PRs related to dependencies. Meta: Refactor Issues and PRs related to refactors. Meta: Typings Issues and PRs related to typings. Mod: SettingsGateway Issues and PRs related to SettingsGateway. SEM: Major PRs that contain breaking changes and should be released in the next major version. Status: Needs Testing PRs that need testing from the author or volunteers. Type: Enhancement Issues and PRs related to feature enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants