Make learning materials page CRUD using the Rails Admin gem #87

Nemwel-Boniface · 2024-04-17T17:49:39Z

Hello there 👋

This pull request works on part of the requirements in this issue #78 . I tried to stick as much as possible to the provided design guidelines.

The specific tasks implemented in this pull request include:

1️⃣ Setup of the Rails Admin gem to the project which provides a dashboard. The dashboard allows a community admin to create, edit, and delete records. and Learning materials (and other resources as well in the project.). The following sub-sections show the process of creating, editing, and deleting a learning material.
a. Creating a learning material record from the Rails Admin dashboard

Screencast.from.17-04-2024.08.36.05.ALASIRI.webm

 b. Updating a learning material record from the Rails Admin dashboard

Screencast.from.17-04-2024.08.37.17.ALASIRI.webm

 c. Deleting a learning material record from the Rails Admin dashboard

Screencast.from.17-04-2024.08.38.14.ALASIRI.webm

The screencast below shows what the new learning materials page looks like:

Screencast.from.17-04-2024.08.41.19.ALASIRI.webm

2️⃣ Authorisation has been implemented using the cancancan gem which makes sure that only a community admin with the role "organization_admin" can access the dashboard and make any necessary changes.
3️⃣ Updated the routes including the /admin and /learning_materials routes to allow access to the new features
4️⃣ Created a learning_materials/index.html.erb view which holds the markup for the new learning page. The view is designed using TailwindCSS and is mobile responsive as well.

Other changes made that were not part of the core requirements:

1️⃣ In the stylesheets/mailgun_mails.css file, I noticed that there are repetitions of some color codes. I therefore made some CSS variables to reduce these many repetitions.
2️⃣ Added Rspec and Capybara for some integration tests.
In the http://127.0.0.1:3000/learning_materials I added a section that lists the learning materials showing the Learning material title, description, and the link. This will help those who do not want to view the cards to view the list for an easier view.

How to test the changes made:

1️⃣ To best the new learning materials page, navigate to this page http://127.0.0.1:3000/learning_materialsvor optionally click on any link that has "Learning Materials```.
2️⃣ To navigate to the admin dashboard:
a. Ensure that you have an admin account. You can create an account and edit it in the dashboard under "users". Make sure that the account role is organization_admin. To do this you might need to first disable lines 45-49 in the config/initializers/rails_admin.rb file. Then un comment it afterward to test other bits of the changes made.
b. Once you have an account that has the organization admin role, you can now navigate to http://127.0.0.1:3000/admin after you have successfully authenticated yourself.
To run the tests run bundle exec rspec spec/features/learning_materials_spec.rb.

Features not yet implemented in this pull request:

Having a look at the provided design guidelines, the feature for "searching" to filter the available learning materials has not yet been implemented. This could be implemented as a feature improvement of the existing code.

Slight bug known which does not affect the working of the feature implemented:

When you access the learning materials resource, you will notice two "thumbnails". One as an input box and one as an image upload. This is caused because:

I have the thumbnails column as a string (which shows the input form for text in the rails admin dashboard).
In the learning materials model, I have the has_one_attached association which creates an Active Record attachment which creates the other thumbnail.

Therefore when creating the learning materials from the admin dashboard you can safely ignore the one that shows as an input box field. I would appreciate suggestions to make the thumbnail show as one and not two in the admin dashboard.

Thank you for taking the time to check out my PR 🙏

…admin route

- Create the learning materials resource - Migrate the learning materials resource to update the schema file

- Update routes with the admin route and the learning materials routes - Add associations between User and Learning materials models - Create Learning materials controller with index action

- Add CSS variables to reduce the number of repetitions of color codes - Add the default thumbnail that will display when no image is uploaded when learning material is created - Create the learning materials index view listing all the currently available learning materials

- Update all links to the learning materials page with the correct updated one (learning_materials_path). - Add authorization rules to the rails admin config to only allow users with role "organization_admin" access the dashboard.

- Setup rspec in the project for unit tests - Configured Capybara to work with rspec in the spec_helper.rb file for integration/ feature tests

- Write integration test to the application - Update learning materials code moving logic to the controller moving it from the views

- Add a section that lists the currently available learning materials for people who do not want to view the cards listed above - Add validations in the learning materials model for the title, description and link - Refactor error message localization in LearningMaterial model to improve code maintainability

JudahSan · 2024-04-18T21:51:13Z

Great work @Nemwel-Boniface.
Kindly resolve conflicts.

Nemwel-Boniface · 2024-04-19T11:35:36Z

Thank you @JudahSan I am working on it. 👍

denissellu

@Nemwel-Boniface this is amazing work, thanks! 🌟

Do we prefer rails_admin over motor-admin?

if so should we remove Motor admin and it's config? so there is no confusion for the future?

arc_platform/config/routes.rb

Lines 4 to 7 in f4e6447

    
           mount RailsAdmin::Engine => '/admin', as: 'rails_admin' 
        
           authenticate :user do 
        
             mount Motor::Admin => '/admin' 
        
           end

Nemwel-Boniface · 2024-04-22T18:24:22Z

Hello @denissellu 👋

I preferred to go with Rails Admin over Motor Admin because of the following key reasons:

Rails Admin is well established and mature with a wider community using it than Motor Admin. In case of issues, We are guaranteed that it has probably been covered/ discussed earlier and resolved.
When comparing Rails Admin with Motor Admin, Rails Admin is more customizable which allows you to adjust it to your specific needs with ease.

With the above put into consideration, I would strongly recommend removing Motor Admin along with its config. Should I proceed to do so?

Thank you for having a look at my PR! 🙏

denissellu · 2024-04-22T19:23:11Z

@Nemwel-Boniface interesting, I've not used rails_admin on a large scale project, so i don't really have a preference! shall we open this to the others? @banta and @JudahSan thoughts?

denissellu

@Naphtali-cpu I've gone through the code, please find my suggested changes below

a few things that can be improved on:

if we are sticking with Rspec, It would be good to add some request tests for the LearningMaterialsController. These tests will help us make sure the controller is handling requests properly and giving back the right responses, it covers the whole stack, which isn't as thoroughly tested with just a feature tests
Also, lets use FactoryBot to set up our test data properly, it's great for when we need lots of different test scenarios. For example in the LearningMaterialsController#index you have a few things happening, displaying all learning materials, filtering those with thumbnails, then selecting 2 randomly, all of which will be easy to setup the right scenarios and test it!

good job! i like a lot of these changes!

app/controllers/learning_materials_controller.rb

denissellu · 2024-04-22T20:17:47Z

app/controllers/learning_materials_controller.rb

+      learningmaterial.thumbnail.attached?
+    end
+    @random_learningmaterials = @learningmaterials_with_thumbnails.sample(2)
+    @learningmaterials_with_or_without_thumbnails = @learningmaterials - @random_learningmaterials


Suggested change

@learningmaterials_with_or_without_thumbnails = @learningmaterials - @random_learningmaterials

@learning_materials_without_random = @learning_materials - @random_learning_materials

maybe this makes more sense? 🤷‍♂️ naming things is hard 😅

Thank you for this suggestion and yes naming things is really hard because there is never a "one right/ true way" to name things. The reason why it is named as "with or without thumbnails" is because it is supposed to hold all other Learning materials regardless of whether that specific learning material has an attached thumbnail or not. If this is renamed to your suggestion I am afraid it will lead to slight confusion later.

What do you think @denissellu ? 🤔

denissellu · 2024-04-22T20:21:43Z

Gemfile

@@ -66,7 +66,11 @@ group :test do
  # Use system testing [https://guides.rubyonrails.org/testing.html#system-testing]
  gem 'capybara'
  gem 'faker', '~> 3.1'
+  gem 'rspec-rails', '~> 6.1'


i myself prefer Rspec too, if we are going with Rspec, then we need to remove minitest, or make it clear which one we should be using on the project! @JudahSan @banta should we keep Rspec over Minitest?

btw i'm aware that replacing minitest with rspec will also mean updating the github action to actually run the tests and move any minitest already written

Looking forward to your opinions on this @banta and @JudahSan

config/routes.rb

- Use recommended snake case format for naming my variables. - Eagerload thumbnails for the Learning materials to avoid N + 1 queries.

- Remove Motor admin from the project along with its config files - Test application to make sure it works still as required after removing motor admin

Nemwel-Boniface added 9 commits April 15, 2024 22:17

Update Setup of the rails_admin gem in the project accessed through /…

e8acb37

…admin route

Update include:

8717427

- Create the learning materials resource - Migrate the learning materials resource to update the schema file

Update include:

a6d167f

- Update routes with the admin route and the learning materials routes - Add associations between User and Learning materials models - Create Learning materials controller with index action

Update include:

f0da3d4

- Update all links to the learning materials page with the correct updated one (learning_materials_path). - Add authorization rules to the rails admin config to only allow users with role "organization_admin" access the dashboard.

Update include:

78753a1

- Setup rspec in the project for unit tests - Configured Capybara to work with rspec in the spec_helper.rb file for integration/ feature tests

Update include:

a2ddd4e

- Write integration test to the application - Update learning materials code moving logic to the controller moving it from the views

Update check and fix all found Rubocop offences

bc13a7f

JudahSan linked an issue Apr 18, 2024 that may be closed by this pull request

Make learning materials page CRUD #78

Open

Nemwel-Boniface marked this pull request as ready for review April 19, 2024 09:05

JudahSan requested review from banta and denissellu April 22, 2024 16:29

denissellu requested changes Apr 22, 2024

View reviewed changes

Update include:

a5b1b1d

- Use recommended snake case format for naming my variables. - Eagerload thumbnails for the Learning materials to avoid N + 1 queries.

JudahSan removed the request for review from banta April 23, 2024 21:55

Update:

a944f54

- Remove Motor admin from the project along with its config files - Test application to make sure it works still as required after removing motor admin

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make learning materials page CRUD using the Rails Admin gem #87

Make learning materials page CRUD using the Rails Admin gem #87

Nemwel-Boniface commented Apr 17, 2024 •

edited

Loading

JudahSan commented Apr 18, 2024

Nemwel-Boniface commented Apr 19, 2024

denissellu left a comment •

edited

Loading

Nemwel-Boniface commented Apr 22, 2024

denissellu commented Apr 22, 2024 •

edited

Loading

denissellu left a comment •

edited

Loading

denissellu Apr 22, 2024

Nemwel-Boniface Apr 23, 2024

denissellu Apr 22, 2024

denissellu Apr 22, 2024

Nemwel-Boniface Apr 23, 2024

	mount RailsAdmin::Engine => '/admin', as: 'rails_admin'
	authenticate :user do
	mount Motor::Admin => '/admin'
	end

	@learningmaterials_with_or_without_thumbnails = @learningmaterials - @random_learningmaterials
	@learning_materials_without_random = @learning_materials - @random_learning_materials

Make learning materials page CRUD using the Rails Admin gem #87

Are you sure you want to change the base?

Make learning materials page CRUD using the Rails Admin gem #87

Conversation

Nemwel-Boniface commented Apr 17, 2024 • edited Loading

Hello there 👋

The screencast below shows what the new learning materials page looks like:

Other changes made that were not part of the core requirements:

How to test the changes made:

Features not yet implemented in this pull request:

Slight bug known which does not affect the working of the feature implemented:

Thank you for taking the time to check out my PR 🙏

JudahSan commented Apr 18, 2024

Nemwel-Boniface commented Apr 19, 2024

denissellu left a comment • edited Loading

Choose a reason for hiding this comment

Nemwel-Boniface commented Apr 22, 2024

Hello @denissellu 👋

denissellu commented Apr 22, 2024 • edited Loading

denissellu left a comment • edited Loading

Choose a reason for hiding this comment

denissellu Apr 22, 2024

Choose a reason for hiding this comment

Nemwel-Boniface Apr 23, 2024

Choose a reason for hiding this comment

denissellu Apr 22, 2024

Choose a reason for hiding this comment

denissellu Apr 22, 2024

Choose a reason for hiding this comment

Nemwel-Boniface Apr 23, 2024

Choose a reason for hiding this comment

Nemwel-Boniface commented Apr 17, 2024 •

edited

Loading

denissellu left a comment •

edited

Loading

denissellu commented Apr 22, 2024 •

edited

Loading

denissellu left a comment •

edited

Loading