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

Tn/add project vars #934

Merged
merged 21 commits into from Oct 2, 2023
Merged

Tn/add project vars #934

merged 21 commits into from Oct 2, 2023

Conversation

TylerNoblett
Copy link
Collaborator

This PR adds project specific vars that can be accessed by a super admin. This is one of three PRs to address this request; the next will add project specific variables and the final one will send both sets of variables to service workers
Screen Shot 2023-09-27 at 11 41 31 AM

Notes

  • only a super admin can create, edit and delete project variables
  • variables aren't required to have a value
  • variable names are unique

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Comment on lines +368 to +374
var newGlobalVar = newGlobalVarGen(1)
var VarExpelliarmus = newGlobalVar("Expelliarmus", "disarm an opponent")
var VarAlohomora = newGlobalVar("Alohomora", "unlock doors")
var VarAscendio = newGlobalVar("Ascendio", "lifts the caster high into the air")
var VarImperio = newGlobalVar("Imperio", "control another person")
var VarLumos = newGlobalVar("Lumos", "creates a narrow beam of light")
var VarObliviate = newGlobalVar("Obliviate", "erases memories")
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

haha I'm all about 1) honoring convention and 2) referencing media franchises from my childhood in my work

Copy link
Collaborator

@JoelAtDeluxe JoelAtDeluxe left a comment

Choose a reason for hiding this comment

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

Still looking at the frontend code, but the backend looks good.

@TylerNoblett
Copy link
Collaborator Author

whoops sorry Joel; I've updated a few things since you last looked - I second guessed some decisions as I was copying a bunch of this stuff over to create operation variables

CREATE TABLE global_vars (
id INT AUTO_INCREMENT,
name VARCHAR(255) NOT NULL UNIQUE,
value VARCHAR(255) NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is VARCHAR(255) large enough? What if* I wanted to pass an RSA key or longer document?

Maybe this isn't a huge deal for the first pass since we can grow the field later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh that's a good point - yeah I can def change that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed the type to TEXT - how does that sound?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just tested by adding a really long doc and the UI 'breaks' so I'll fix that as well

@jkennedyvz jkennedyvz merged commit 26d5492 into main Oct 2, 2023
9 checks passed
@TylerNoblett TylerNoblett deleted the tn/add-project-vars branch October 18, 2023 14:30
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.

None yet

3 participants