-
Notifications
You must be signed in to change notification settings - Fork 229
Fixed Issue #1782, which was changing how the search bar looks in Autolab #2292
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
base: master
Are you sure you want to change the base?
Conversation
…search bar with the appropriate library
This reverts commit 184e4c2.
📝 Walkthrough""" WalkthroughA custom search bar was added above the gradesheet table, replacing the built-in DataTables search input. Supporting JavaScript was updated to connect the new input to table filtering, and new CSS styles were introduced for the search bar. The database schema was updated with new tables and columns related to rubric items and assignments, and other schema adjustments. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant DataTable
User->>Browser: Type in custom #filter input
Browser->>DataTable: Filter table rows based on input value
Note right of DataTable: Table updates to show filtered results
User->>Browser: Press Enter or Escape in #filter input
Browser->>DataTable: (Enter) Focus specified element\n(Escape) Clear input and prevent default
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/views/assessments/viewGradesheet.html.erb (1)
283-288
: Improve accessibility of the search input.The search input has been properly added with appropriate styling, but missing screen reader accessibility attributes.
<div class="filter_container search-student-bar"> <i class="material-icons prefix">search</i> <div style="width: 30%;"> - <label for="filter"></label><input id="filter" placeholder="Search..." type="text" autocomplete="off"> + <label for="filter" class="sr-only">Search students</label><input id="filter" placeholder="Search..." type="text" autocomplete="off" aria-label="Search students"> </div> </div>Consider adding a
.sr-only
class in CSS for screen-reader only elements if not already available.db/schema.rb (1)
14-15
: Fix minor style issueThere's an extra empty line after the schema definition opening that should be removed according to your code style conventions.
ActiveRecord::Schema.define(version: 2025_04_26_203028) do - create_table "active_storage_attachments", force: :cascade do |t|
🧰 Tools
🪛 RuboCop (1.73)
[convention] 14-15: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Gemfile.lock
is excluded by!**/*.lock
,!**/*.lock
libsqlite3.dylib
is excluded by!**/*.dylib
,!**/*.dylib
📒 Files selected for processing (5)
Gemfile
(1 hunks)app/assets/javascripts/gradesheet.js.erb
(2 hunks)app/assets/stylesheets/gradesheet.css.scss
(1 hunks)app/views/assessments/viewGradesheet.html.erb
(1 hunks)db/schema.rb
(10 hunks)
🧰 Additional context used
🪛 RuboCop (1.73)
db/schema.rb
[convention] 14-15: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
🔇 Additional comments (10)
Gemfile (1)
52-52
:❓ Verification inconclusive
Verify this version constraint is necessary.
The change from a flexible version constraint (
~> 0.5
) to an exact version (= 0.5.4
) for mysql2 gem is very restrictive and could impact dependency resolution during future upgrades.
🏁 Script executed:
#!/bin/bash # Check if there are any issues or compatibility requirements documented for this specific version grep -r "mysql2.*0.5.4" --include="*.{md,txt,rb}" . # Check commit history for any related changes git log -n 5 --grep="mysql2"Length of output: 53
🏁 Script executed:
#!/bin/bash # Inspect the locked mysql2 version in Gemfile.lock grep -R "mysql2 " Gemfile.lock # Show the surrounding lines for context grep -R -n -C2 "mysql2 " Gemfile.lockLength of output: 164
Gemfile: Confirm mysql2 version constraint necessity
I wasn’t able to find any docs, commit notes, or a Gemfile.lock entry mandating exactly 0.5.4. Locking the mysql2 gem to
= 0.5.4
is very restrictive and may make future upgrades harder. Please verify:
- That a Gemfile.lock exists and lists mysql2 at 0.5.4.
- Any upstream or project-specific compatibility notes requiring exactly 0.5.4.
app/assets/stylesheets/gradesheet.css.scss (1)
18-27
: Nice addition of a styled search bar.The flexbox styling creates a well-aligned search input that's consistent with modern web design practices. The height of 3rem ensures good touch target size for the input element.
app/assets/javascripts/gradesheet.js.erb (2)
121-121
: Removed built-in DataTables search filter.The 'sDom' parameter has been changed to remove the built-in DataTables search filter ('f'), keeping only the table ('t'). This appropriately aligns with the custom search implementation.
150-162
: Well-implemented custom filter functionality.The event handlers for the new search input are correctly implemented:
keyup
event filters the table as the user typeskeydown
event handles Enter key (to focus another element) and Escape key (to clear the input)This implementation maintains all the functionality of the original search while improving the UI.
db/schema.rb (6)
57-58
: LGTM: Adding rubric_item_id to annotationsThe addition of
rubric_item_id
to theannotations
table with a corresponding index is a good enhancement that allows annotations to be linked to specific rubric items. This supports the new rubric-based grading functionality.
230-231
: LGTM: Adding datetime precisionAdding precision to datetime fields in
lti_course_data
is a good practice for more accurate timestamp storage.
324-333
: LGTM: New rubric_item_assignments tableThe new
rubric_item_assignments
table is well-structured with appropriate indexes and foreign keys to enable the association between rubric items and submissions. Theassigned
boolean field will be useful to track assignment status.
335-344
: LGTM: New rubric_items tableThe new
rubric_items
table is well-designed with:
- Appropriate foreign key to
problems
- Required fields for description, points, and order
- A composite unique index on problem_id and order to ensure proper sequencing
This table will support the new rubric-based grading feature effectively.
464-471
: LGTM: Adding foreign key constraintsThe addition of foreign key constraints enhances data integrity by ensuring:
- Annotations refer to valid rubric items
- GitHub integrations are linked to valid users
- OAuth relationships maintain referential integrity
- Rubric items are associated with valid problems
- Rubric item assignments link to valid submissions and rubric items
This change helps maintain data consistency across the application.
13-472
:❓ Verification inconclusive
Verify search bar UI changes aligned with the schema changes
The PR states it addresses "changing how the search bar looks in Autolab," but the schema changes are extensive and include new rubric functionality. Ensure that the schema changes don't introduce unexpected side effects to the search bar UI changes that are the focus of this PR.
🌐 Web query:
What is the relationship between database schema changes and UI components in Rails applications?
💡 Result:
Relationship Between Database Schema Changes and UI Components in Rails Applications
The relationship between database schema changes and UI components in Rails applications is tightly coupled through the framework’s conventions and architecture, particularly its use of the Model-View-Controller (MVC) pattern and Active Record ORM.
How Schema Changes Connect to UI Components
Database Schema as Data Source
In Rails, the database schema—defined and modified through migrations—dictates the structure of the application's data: tables, columns (attributes), relationships, and constraints[1][5][8]. This schema forms the foundation for Active Record models, which represent business entities and are directly mapped to database tables.Active Record and the UI
Active Record models interact with the database using the current schema and expose this data to controllers and views. UI components (views, forms, etc.) consume this data, display it to users, and send changes back to the models through controllers[8]. Therefore, any change in the database schema—such as adding a new field or changing a data type—directly affects what data is available to the UI and how it is processed and displayed.Practical Example
If you add a new column (e.g.,price
to aproducts
table) via a migration, you must update:
- The Active Record model (if validations or methods rely on this field)
- The form views (to allow users to enter or edit the new attribute)
- The index or show views (to display the new attribute)[1][5]
This chain illustrates how a schema change propagates to the UI layer.Coupled Deployment Patterns
Rails projects often deploy code and schema changes together, especially in small teams or monolithic applications[3]. If the schema and code/UI become misaligned (for example, deploying UI code that expects a column that does not exist yet), runtime errors or user-facing issues can arise. For large or complex systems, decoupled (carefully sequenced) deployments of schema and UI code are sometimes necessary to avoid outages or inconsistencies[3].Maintaining Consistency and Avoiding Errors
It’s essential to coordinate schema changes and updates to UI components. Inconsistent states can lead to:
- UI errors (e.g., forms break if a needed column is missing)
- Data inconsistencies (e.g., UI submits fields not present in the schema)[4][3]
Good practices include using migrations to track schema changes, updating models and UI components in tandem, and deploying them together or in a carefully orchestrated sequence[1][5][7].Impact of Upgrades and Framework Changes
Framework upgrades (e.g., Rails 6.1 to 7.0) may introduce schema file changes, requiring alignment between schema version, model code, and UI expectations to avoid mismatches[7].Summary Table
Aspect Effect of Schema Change on UI Adding a column Must update forms, views, and possibly model validations/methods Removing a column Must remove from UI to avoid errors Changing data type May need to adjust UI formatting or input handling Adding relationships Update associated forms & views Renaming fields Update all references in models, controllers, and UI Conclusion
In Rails applications, database schema changes and UI components are inherently linked. Schema migrations drive what data is available to the application; models reflect schema structure; UI components present and collect data based on model definitions. Therefore, evolving one side (schema) usually requires thoughtful, corresponding updates on the other (UI), and careful deployment practices to maintain system integrity and a good user experience[1][5][3][4][7][8].
Citations:
- 1: https://guides.rubyonrails.org/getting_started.html
- 2: https://www.bennadel.com/blog/4191-i-always-design-the-database-schema-first-then-the-coldfusion-code.htm
- 3: https://www.bytebase.com/blog/how-to-handle-database-schema-change/
- 4: https://www.metaplane.dev/blog/database-schema-changes
- 5: https://guides.rubyonrails.org/active_record_migrations.html
- 6: https://www.mikecoutermarsh.com/no-fear-schema-changes-planetscale_rails-gem/
- 7: https://www.fastruby.io/blog/dealing-with-schema-changes-in-rails-7.html
- 8: https://guides.rubyonrails.org/active_record_basics.html
Verify UI–DB alignment for search bar changes
The PR bundles extensive schema migrations for rubric support with cosmetic updates to the search bar. Because Rails views and controllers rely on ActiveRecord models and their columns, please confirm that the search bar’s behavior and rendering remain correct after applying these schema changes.
- Review
SearchController
(or equivalent) to ensure permitted params and query scopes still reference existing columns.- Inspect any model scopes or methods used by the search bar—verify they don’t rely on renamed/removed attributes.
- Check the search bar view partials (e.g.,
_search.html.erb
) and associated JavaScript for selectors or data-attributes tied to column names.- Run a quick smoke test of the search feature in your local/dev environment post-migration to catch any runtime errors.
🧰 Tools
🪛 RuboCop (1.73)
[convention] 14-15: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
t.text "banner", limit: 65535 | ||
t.text "colspec", limit: 65535 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify text field limit changes
Several text fields have had their limits specified or modified:
banner
andcolspec
inscoreboards
(limit: 65535)feedback
inscores
(limit: 16777215)dave
insubmissions
(limit: 255)
Ensure that existing data fits within these limits and that the application logic accounts for these constraints.
Also applies to: 373-373, 399-399
🏁 Script executed:
#!/bin/bash
# Check the migrations that modified these text field limits
echo "Checking for migrations that modify text field limits:"
rg -A 3 -B 3 "limit:" --glob="db/migrate/*.rb" | grep -E "banner|colspec|feedback|dave"
Length of output: 1338
Ensure text field limits are enforced and existing data fits
Although the migrations setting these limits are already in place (see 20150127210917, 20150406221712, 20150119232050), you should:
• Run queries against your production data to verify no rows exceed the new limits (e.g.
SELECT MAX(CHAR_LENGTH(feedback)) FROM scores;
,
SELECT MAX(CHAR_LENGTH(banner)), MAX(CHAR_LENGTH(colspec)) FROM scoreboards;
,
SELECT MAX(CHAR_LENGTH(dave)) FROM submissions;
)
• Add or update ActiveRecord validations to mirror the DB constraints:
– In Score model: validates :feedback, length: { maximum: 16_777_215 }
– In Scoreboard model: validates :banner, :colspec, length: { maximum: 65_535 }
– In Submission model: validates :dave, length: { maximum: 255 }
• Augment or add model-level tests to assert these validations fire when limits are exceeded.
This will prevent runtime errors and data truncation when new records are created or updated.
🤖 Prompt for AI Agents (early access)
In db/schema.rb around lines 365 to 366, the text fields banner and colspec have limits set to 65535. To fix the issue, first run SQL queries on production data to ensure no existing entries exceed these limits. Then, add or update ActiveRecord validations in the corresponding models: add length validations for banner and colspec in the Scoreboard model, feedback in the Score model, and dave in the Submission model to match their database limits. Finally, create or update model tests to verify these validations trigger correctly when limits are exceeded.
@@ -130,7 +132,7 @@ | |||
t.integer "course_id" | |||
t.integer "assessment_id" | |||
t.string "category_name", default: "General" | |||
t.datetime "release_at", default: -> { "CURRENT_TIMESTAMP" } | |||
t.datetime "release_at" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify removal of default value for release_at
The release_at
field in attachments
table appears to have had its default value removed. Ensure this change won't affect existing functionality that might rely on a default value.
🏁 Script executed:
#!/bin/bash
# Check the migration that modified this column
echo "Checking for migrations that modify the release_at column:"
rg -A 3 -B 3 "release_at" --glob="db/migrate/*.rb"
Length of output: 1455
Action Required: Confirm and Formalize Removal of release_at
Default
The schema now shows:
t.datetime "release_at"
but the original migration
db/migrate/20230924161219_add_release_at_to_attachments.rb
added this column with
default: -> { 'CURRENT_TIMESTAMP' }
I did not find any subsequent migration using change_column_default
(or similar) to drop that default. Please:
- Verify whether you intended to remove the
CURRENT_TIMESTAMP
default. - If so, add and run a new migration (e.g.
change_column_default :attachments, :release_at, from: -> { 'CURRENT_TIMESTAMP' }, to: nil
) so the schema and DB stay in sync. - If not, revert the schema change to include the original default.
- After you decide, ensure no existing code or tests assume
release_at
is non‐NULL (update creation logic or test fixtures as needed).
🤖 Prompt for AI Agents (early access)
In db/schema.rb at line 135, the release_at column in the attachments table no longer has the default CURRENT_TIMESTAMP, but the original migration set this default. Verify if you intended to remove this default; if yes, create and run a new migration explicitly changing the default from CURRENT_TIMESTAMP to nil to keep the schema and database consistent. If not, revert the schema change to restore the default. Also, review and update any code or tests that assume release_at is non-NULL to prevent issues.
t.integer "record_id", null: false | ||
t.integer "blob_id", null: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider implications of changing from bigint
to integer
for ID columns
Several columns in Active Storage tables have been changed from bigint
to integer
:
record_id
andblob_id
inactive_storage_attachments
byte_size
inactive_storage_blobs
blob_id
inactive_storage_variant_records
This reduces the maximum value that can be stored (from ~9.2 quintillion to ~2.1 billion). Ensure this won't cause issues with large files or in systems with many records.
Also applies to: 30-30, 38-38
🏁 Script executed:
#!/bin/bash
# Check if there are any migrations that explicitly change these column types
echo "Checking for migrations that modify Active Storage column types:"
rg -A 3 -B 3 "change_column.*active_storage" --glob="db/migrate/*.rb"
Length of output: 992
Let’s verify the original Active Storage migrations to see what column types were defined there:
🏁 Script executed:
#!/bin/bash
echo "Inspecting the Active Storage attachments migration:"
rg -A5 -B5 "create_table :active_storage_attachments" --glob="db/migrate/*.rb"
echo -e "\nInspecting the Active Storage blobs migration:"
rg -A5 -B5 "create_table :active_storage_blobs" --glob="db/migrate/*.rb"
echo -e "\nInspecting the Active Storage variant records migration:"
rg -A5 -B5 "create_table :active_storage_variant_records" --glob="db/migrate/*.rb"
Length of output: 4277
Action Required: Address Active Storage column type mismatch
The schema dump now shows these columns as integer
, but the original migrations defined them as bigint
and there are no migrations to change their types. Please confirm whether you intend to down-cast these columns or revert them to bigint
. If the change is intentional, add migrations to alter the column types; otherwise, restore the original types in db/schema.rb
.
Locations needing attention:
db/schema.rb:18-19
• active_storage_attachments:record_id
,blob_id
db/schema.rb:30
• active_storage_blobs:byte_size
db/schema.rb:38
• active_storage_variant_records:blob_id
🤖 Prompt for AI Agents (early access)
In db/schema.rb around lines 18-19, 30, and 38, the columns record_id and blob_id in active_storage_attachments, byte_size in active_storage_blobs, and blob_id in active_storage_variant_records are defined as integer but were originally bigint in the migrations. To fix this, verify if the change to integer is intentional; if not, revert these column types back to bigint in db/schema.rb. If the change is intended, create new migrations to explicitly alter these columns from bigint to integer to keep schema and database consistent.
@@ -10,13 +10,13 @@ | |||
# | |||
# It's strongly recommended that you check this file into your version control system. | |||
|
|||
ActiveRecord::Schema.define(version: 2024_04_06_174050) do | |||
ActiveRecord::Schema.define(version: 2025_04_26_203028) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify schema version timestamp
The schema version is set to 2025_04_26_203028
, which contains a future year (2025). While this might be intentional for development purposes, it's unusual to have a timestamp in the future.
🏁 Script executed:
#!/bin/bash
# Check other migration files to see if this timestamp pattern is consistent
echo "Checking migration file timestamps:"
fd -e rb -p "db/migrate/.*\.rb" | sort | tail -5
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Search for migrations with 2025 prefix
echo "Migrations with 2025 prefix:"
fd -e rb db/migrate | grep 'db/migrate/2025_' || echo 'None found'
Length of output: 489
🏁 Script executed:
#!/bin/bash
echo "Searching for migration file matching schema version 2025_04_26_203028..."
find db/migrate -maxdepth 1 -type f -name "2025_04_26_203028*.rb" || echo "No matching migration file found."
Length of output: 224
Migration Timestamp Mismatch in db/schema.rb
The schema version in db/schema.rb
is set to 2025_04_26_203028
, but there is no corresponding migration file (db/migrate/2025_04_26_203028*.rb
) in the repo. This suggests the migration wasn’t committed or the timestamp is incorrect.
• File: db/schema.rb
(line 13)
ActiveRecord::Schema.define(version: 2025_04_26_203028) do
• Verification:
$ find db/migrate -maxdepth 1 -name "2025_04_26_203028*.rb"
# => no matching migration found
Please ensure one of the following:
- Commit the missing migration file with timestamp
2025_04_26_203028
- Run your migrations locally and regenerate
schema.rb
so the version matches the latest migration - Manually correct the schema version to match the most recent migration in
db/migrate/
🤖 Prompt for AI Agents (early access)
In db/schema.rb at line 13, the schema version is set to 2025_04_26_203028, but no migration file with this timestamp exists in db/migrate/. To fix this, either commit the missing migration file with that timestamp if it exists, or run your migrations locally and regenerate schema.rb so the version matches the latest migration. Alternatively, manually update the schema version in schema.rb to match the most recent migration timestamp found in db/migrate/.
We created a fix for the misaligned and slightly ugly search bars in Autolab
Description
Changed the following Files:
gradesheet.js.erb
viewGradesheet.html.erb
gradesheet.css.scss
to change how the search bar is rendered when searching through the grade sheet. This is reflected in these changes:

Old:
New:

Motivation and Context
This change is required because it helps standardize search bars among all Autolab pages. This increases consistency in the UI as well as increase filtering speeds for people using the search bar. The open issue that it fixes is #1782
How Has This Been Tested?
I tested it by looking at the interfaces and saw that the search bar changed what it looked like. The search bar is also tested to work properly by filtering properly. This is seen as in the screenshots above. I also passed all the linters in Autolab.
Types of changes
Checklist:
overcommit --install && overcommit --sign
to use pre-commit hook for lintingOther issues / help required
N/A