Skip to content
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

Developed Projects listing page #105

Merged
merged 8 commits into from
Feb 2, 2022
Merged

Developed Projects listing page #105

merged 8 commits into from
Feb 2, 2022

Conversation

judis007
Copy link
Contributor

@judis007 judis007 commented Feb 1, 2022

Pull Request Template

Description

Developed the Project listing page which shows the projects and the number of hours logged for that project. It also includes a search bar to search for the projects.

Fixes #82

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I tested locally in my browser whether the required functionality and design is working.

Checklist:

  • I have manually tested all workflows
  • I have performed a self-review of my own code
  • I have added automated tests for my code

@vipulnsward vipulnsward temporarily deployed to miru-review-pr-105 February 1, 2022 16:22 Inactive
@judis007 judis007 changed the title Developed Projects listing page [WIP] Developed Projects listing page Feb 1, 2022
@pr-triage pr-triage bot removed the PR: unreviewed label Feb 1, 2022
Copy link
Contributor

@keshavbiswa keshavbiswa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some non blocking concerns.

class ProjectsController < ApplicationController
def index
if params["query"]
@projects = Project.where("name LIKE ?", "%#{params['query']}%")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be really nice if we could use Ransack gem for the search part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keshavbiswa, I have added the Ransack gem. Can you review it once again?

app/controllers/projects_controller.rb Outdated Show resolved Hide resolved
@judis007 judis007 changed the title [WIP] Developed Projects listing page Developed Projects listing page Feb 2, 2022
@vipulnsward vipulnsward temporarily deployed to miru-review-pr-105 February 2, 2022 06:44 Inactive
@judis007 judis007 requested a review from keshavbiswa February 2, 2022 06:46
Copy link
Contributor

@keshavbiswa keshavbiswa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🥂

class ProjectsController < ApplicationController
def index
@q = Project.ransack(params[:q])
@projects = @q.result(distinct: true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check why am I seeing these extra params
Screenshot 2022-02-02 at 12 41 51 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohitjoshixyz That is from the image_submit_tag. It will include the x and y params. I referred some posts. But I didn't find any solution to get rid of that.

PFA
https://stackoverflow.com/questions/21685559/rails-image-submit-tag-x-y-values

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay its fine, not a blocker will check it later

akhilgkrishnan and others added 3 commits February 2, 2022 15:47
* Code quality improvements

* locales added

* minor code changes

* review changes added

* review changes added
@vipulnsward vipulnsward temporarily deployed to miru-review-pr-105 February 2, 2022 10:18 Inactive
@vipulnsward vipulnsward temporarily deployed to miru-review-pr-105 February 2, 2022 10:19 Inactive
@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Current Code Coverage Percent of this PR:

71.84 %

Files having coverage below 100%

Impacted Files Coverage
/config/application.rb 92.86 %
/config/routes.rb 94.74 %
/app/models/user.rb 83.33 %
/app/controllers/projects_controller.rb 50.0 %
/app/controllers/application_controller.rb 66.67 %
/app/helpers/application_helper.rb 40.0 %
/app/helpers/team_helper.rb 57.14 %
/app/helpers/users/registrations_helper.rb 41.67 %
/app/controllers/company_controller.rb 36.67 %
/app/controllers/root_controller.rb 33.33 %
/app/controllers/team_controller.rb 83.33 %
/app/controllers/users/omniauth_callbacks_controller.rb 25.0 %
/app/controllers/users/sessions_controller.rb 25.0 %
/app/controllers/users/registrations_controller.rb 45.83 %
/app/controllers/users/invitations_controller.rb 63.64 %
/app/controllers/internal_api/v1/application_controller.rb 80.0 %
/app/controllers/internal_api/v1/clients_controller.rb 66.67 %

Copy link
Member

@akhilgkrishnan akhilgkrishnan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@judis007 added some minor comments

app/views/projects/index.html.erb Outdated Show resolved Hide resolved
app/controllers/projects_controller.rb Outdated Show resolved Hide resolved
@vipulnsward vipulnsward temporarily deployed to miru-review-pr-105 February 2, 2022 12:18 Inactive
Copy link
Member

@akhilgkrishnan akhilgkrishnan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 . Can add some projects to seed.rb

@judis007 judis007 merged commit 022e01a into develop Feb 2, 2022
@judis007 judis007 deleted the project-list branch February 2, 2022 12:45
Copy link
Contributor

@rohitjoshixyz rohitjoshixyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Develop the Project list Page
5 participants