[FEAT] Custom Links in Dropdown Menu #5718
Conversation
nepaakash
requested review from
compwron,
FireLemons and
littleforest
as code owners
app/models/learning_hour.rb
Outdated
| @@ -52,7 +52,6 @@ def user_org_learning_topic_enable? | |||
| # id :bigint not null, primary key | |||
| # duration_hours :integer not null | |||
| # duration_minutes :integer not null | |||
| # learning_type :integer default(5) | |||
There was a problem hiding this comment.
interesting diff - at some point we need to figure out why this keeps appearing and disappearing
There was a problem hiding this comment.
Agreed. This thing drives me nuts in my code. Also I feel like I messed it up. @elasticspoon any ideas?
There was a problem hiding this comment.
I think this should be removed. It looks like :learning_type was adding in
It probably should have been removed but somehow got left in?
I suspect what is going on is it exists in the schema because someone forgot to commit the removal. Most people do a schema:load to they have it. But when people run migrations it attempts to remove the column. And every time the removal is attempted we think "thats weird, we should leave the column".
There was a problem hiding this comment.
I would leave it in for now simply because I think the removal might mess up the seeding.
There was a problem hiding this comment.
@nepaakash revert this change for now
| require 'rails_helper' | ||
|
|
||
| RSpec.describe CustomLink, type: :model do | ||
| pending "add some examples to (or delete) #{__FILE__}" |
There was a problem hiding this comment.
please add tests before merging
There was a problem hiding this comment.
Yes sure... It is still a work in progress
app/models/custom_link.rb
Outdated
| class CustomLink < ApplicationRecord | ||
| belongs_to :casa_org | ||
| end |
There was a problem hiding this comment.
Should add some validation on the url and the text
There was a problem hiding this comment.
This task is still in progress... Will definitely add those
db/schema.rb
Outdated
| @@ -420,7 +429,6 @@ | |||
|
|
|||
| create_table "learning_hours", force: :cascade do |t| | |||
| t.bigint "user_id", null: false | |||
| t.integer "learning_type", default: 5 | |||
There was a problem hiding this comment.
revert this change for now
|
@nepaakash how are you coming along here? :) do you need any help? |
|
hi @nepaakash - I hope you are well. CASA stakeholders asked about this yesterday. Anything else needed on this ticket to get it completed? |
Hey @bcastillo32 , RSpecs is remaining in this |
|
I am excited for this to get merged, feel free to ask me any questions |
hi @nepaakash - how are you coming along - our stakeholders are keen on using this feature :) |
Will complete the task by the end of this week |
Hi @nepaakash any progress on this. Our casa stakeholders are inquiring :) |
nepaakash
changed the title
Hello @bcastillo32 everything is done here but the rspec is failing. I checked and it is failing in the main branch too. Cam you look into it? |
|
closing |
What github issue is this PR for, if any?
Resolves #5704