Add column 2 org profiles #289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kasugaijin
reviewed
Oct 19, 2023
app/models/organization_profile.rb
Outdated
| @@ -30,6 +31,7 @@ class OrganizationProfile < ApplicationRecord | |||
| before_save :normalize_phone | |||
|
|
|||
| validates :phone_number, phone: {possible: true, allow_blank: true} | |||
| validates :about_us, presence: true | |||
There was a problem hiding this comment.
can we for now not validate the presence of this? It will mean we have to provide more information when creating a new org, which is done by the console, and already has a fair bit of data required (also why I added allow_blank to the phone attribute also).
My thinking is that an engineer creates a new org and its associations via the console with bear bones details, and then the admin can update the profile details later in their own time.
There was a problem hiding this comment.
Agreed with @kasugaijin here.
My rule about adding validations is to ask wither or not the record is valid if it is missing this the attribute in question. A profile could still be legit if it is missing a about_us section in my mind. In other cases maybe there are attributes that are so important that if its missing we shouldn't save those records. A common one is a User without a email if we rely on the email heavily.
kasugaijin
approved these changes
Oct 19, 2023
edwinthinks
reviewed
Oct 20, 2023
| @@ -0,0 +1,5 @@ | |||
| class AddAboutUs2OrganizationProfiles < ActiveRecord::Migration[7.0] | |||
| def change | |||
| add_column :organization_profiles, :about_us, :string, null: false | |||
There was a problem hiding this comment.
Suggested change
| add_column :organization_profiles, :about_us, :string, null: false | |
| add_column :organization_profiles, :about_us, :string |
In case we want to remove this field's validation.
|
@all-contributors |
nsiwnf
reviewed
Oct 20, 2023
| @@ -1,5 +1,5 @@ | |||
| class AddSpecies2Pets < ActiveRecord::Migration[7.0] | |||
| def change | |||
| add_column :pets, :species, :integer, null: false | |||
There was a problem hiding this comment.
This is an existing migration right? I though that this was already in main
There was a problem hiding this comment.
this is true... how the heck did that get added again?
its me hi, Im the problem its me
…t-rescue into add_column_2_org_profiles
nsiwnf
approved these changes
Oct 21, 2023
ErinClaudio
pushed a commit
that referenced
this pull request
Oct 27, 2023
* Create migration file to add about us to organization profiles table, and add column to seed files * add about us field for organization_profiles * add validation for about us * add validation for about us * standard * Remove validation that we didnt need * remove null false from migration file and migrate * standard rb * add validation for about us * Add back in accidentially removed validation * Migrate updated migration file * Add null false back into correct place and migrate * remove not null comment * standard rb * merge main
kasugaijin
added a commit
that referenced
this pull request
Nov 29, 2023
* Update with partial * Update with attempt with nice_partials gem * Syntax cleanup * Added new nice_partails folder * Syntax cleanup * Update with path params value changed * Update Front-end spacing clean up * Added new partials and Table to DB * Update Changed the task list to now be a modal * Post merge commit * Add Applications list to Pet show (#288) * Update sidebar styling to avoid spilling to tab links Co-authored-by: FionaDL [email protected] * Fix applicant name on applications page Co-authored-by: FionaDL [email protected] * add applications partial and test Co-authored-by: FionaDL [email protected] * add tests for application tab Co-authored-by: FionaDL [email protected] * Clear default_url_options[:script_name] after each test (#287) * Added edit partial * 281: Pets Show Overview Tab make Edit and Delete buttons more visible (#286) * Replace dropdown button with edit button, add delete button beneath summary card * Modify delete button styling * docs: add jadekstewart3 as a contributor for code (#291) * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> * docs: add edwinthinks as a contributor for code (#292) * docs: update README.md [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> Co-authored-by: Ben <[email protected]> * Org dashboard (#290) * Add instance variables that will be used in the dashboard view * Add template html and begin inital formatting to utilize instance variables scoped to organization * remove img tag for non existant avatar * Merge conflict updated * Merge conflict updated * Add column 2 org profiles (#289) * Create migration file to add about us to organization profiles table, and add column to seed files * add about us field for organization_profiles * add validation for about us * add validation for about us * standard * Remove validation that we didnt need * remove null false from migration file and migrate * standard rb * add validation for about us * Add back in accidentially removed validation * Migrate updated migration file * Add null false back into correct place and migrate * remove not null comment * standard rb * merge main * Remove default values from foreign keys (#298) * Improve UI of Accept Invitation Form (#300) * Improve tests stability (#296) * Fix global tests setup * Ensure organization slugs are uniq in tests * Fix factories so they use the same organization for associations * Ensure user emails are uniq in tests * Use current_tenant in factories * Use ActsAsTenant.test_tenant in tests * run standard:fix * fix all contributors table (#293) * 268 part 1: staff can upload profile pic (#276) * Add model changes and tests * Add form field, and changes into the controller * change naming from Picture to Avatar * Run standard fix * Move avatarable tests to shared module * Add uniq index to organization slug (#302) * Merge conflict updated * Update with working but ugly crud features * Syntax error cleanup * Removed binding * Added partial files and corrected before acgtion * Remove 'overview' from the default active tab options * Update Frontend form still nested * Linter error cleanup * Merge conflict resolved * Pet controller refactor * Update with code clean up * Pet controller syntax cleanup * Removed partials * Cleanup Syntax * Test file merge conflict solution * Syntax cleanup * Code refactor * Update spacing in file clean up * Updated frontend with turbo delete button * Update with Features rendering correctly but code is sloppy * Standard error fix * Update spacing and code clean up * Removed unneeded CSS file * Removed more unneeded css * Removed hardcoded org * Added turbo_stream file and last commit before migration that moves away from Tasks * Update with skipped unit tests * Syntax cleanup * Update to spacing * Update spacing cleanup * Update spacing cleanup part 2 * Update spacing and syntax cleanup part 3 * Update styling changes to tasks * Added model tests * Update with passing tests * Update syntax cleanup * Update syntax spacing * Update syntax spacing * Update syntax spacing * Update with Frontend cleanup * Testing all passing * Spacing cleanup * Update cleanup * Update spacing cleanup * Added null:false to tasks name * Added migration file * Added updated test --------- Co-authored-by: aisayo <[email protected]> Co-authored-by: Piotr Borowiec <[email protected]> Co-authored-by: MooseCowBear <[email protected]> Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> Co-authored-by: Ben <[email protected]> Co-authored-by: Jade Stewart <[email protected]> Co-authored-by: Marlena Borowiec <[email protected]> Co-authored-by: Yuri Pains <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 Issue
#285
✍️ Description
Added an about_us column on the organization_profiles table. Updated factories and seeds to reflect the migration. Added validation for about us on organization_profile model, and added validation test.
📷 Screenshots/Demos