WIP: [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? |
Thankyou for asking. Everything is good , Apologies for the delay. Just a quick question, are we not showing any icons for the custom links? |
@nepaakash Thats a good question. We should. You can just use a basic link icon Also, we don't need the soft delete behavior for these links. Soft delete is needed with topics because deletion would impact historical data but that is not the case here. |
|
@nepaakash Getting this when spinning up locally. Probably these references. 
|
@schoork Sorry! I have already fixed that issue locally. I will just push it. |
What github issue is this PR for, if any?
Resolves #5704
What changed, and why?
How is this tested? (please write tests!) 💖💪
Note: if you see a flake in your test build in github actions, please post in slack #casa "Flaky test: " :) 💪
Note: We love capybara tests! If you are writing both haml/js and ruby, please try to test your work with tests at every level including system tests like https://github.com/rubyforgood/casa/tree/main/spec/system
Screenshots please :)
Run your local server and take a screenshot of your work! Try to include the URL of the page as well as the contents of the page.
Feelings gif (optional)
What gif best describes your feeling working on this issue? https://giphy.com/
How to embed:
