From 4591c1dc16b4357e5577772cce022a86e8357165 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 16 Jun 2017 13:22:44 +0530 Subject: [PATCH 01/17] ui changes --- app/assets/javascripts/daily_log.js | 106 +++++++------- app/assets/stylesheets/application.scss | 84 ++++++++---- app/controllers/daily_logs_controller.rb | 3 +- app/models/daily_log.rb | 137 ++++++++++--------- app/views/daily_logs/_user_list.html.haml | 2 +- app/views/daily_logs/_user_summary.html.haml | 4 +- app/views/daily_logs/new.html.haml | 33 +++-- app/views/layouts/application.html.haml | 36 +---- 8 files changed, 210 insertions(+), 195 deletions(-) diff --git a/app/assets/javascripts/daily_log.js b/app/assets/javascripts/daily_log.js index 17d779c..c5fa361 100644 --- a/app/assets/javascripts/daily_log.js +++ b/app/assets/javascripts/daily_log.js @@ -1,58 +1,56 @@ -function DailyLog() { - $(document).ready(function() { - var add_task = $(".add_field_button"); - $(add_task).click(function(e){ - $('.task').append("
") - - }); + function DailyLog() { + $(document).delegate('.add_field_button', 'click', function() + { + $(this).parent().children().eq(1).append("
") + }); + $(document).delegate('.add_project_button', 'click', function() + { + $('.project').append("

Task


"); + var attrid="projects[project_id]"; + $('.added').attr('name', attrid).append($('#projects_project_id').html()); + + }); - var add_project = $(".add_project_button"); - $(add_project).click(function(e){ - $('.project').append("

Task

"); - var attrid="projects[]"; - $('.added').attr('name', attrid).append($('#projects_').html()); - }); - $('input[name="daterange"]').daterangepicker(); -}); -$('input[name="daterange"]').on('apply.daterangepicker', function(ev, picker) { - var project_id= $("#projects_filter").val(); -$('.user_summary').load('/daily_logs/user_logs',{ start_date: picker.startDate.format('YYYY-MM-DD'), end_date: picker.endDate.format('YYYY-MM-DD'),"project_id": project_id}); -}); -$('select').change(alert_me_test); -function alert_me_test(){ - var project_id= $("#projects_filter").val(); -$('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); -} -$('#context1 .menu .item') - .tab({ - context: $('#context1') - }) -; -var today = new Date(); -$('#example1').calendar({ - onChange: function (date, text) { - var newValue = text; - $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); - }, - type: 'date', - maxDate: new Date(today.getFullYear(), today.getMonth(), today.getDate()) - }) -$('.top.menu .item').tab({'onVisible':function(){ - var newValue = $(this).attr("dataval") - $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); -}}); -$('.imageclass').on('click',function(){ - var userid=$(this).attr("id"); - $('.user_list').load('/daily_logs/user_logs',{"user_id":userid}); -}); + $(document).ready(function() { + $('input[name="daterange"]').daterangepicker(); + }); + $('input[name="daterange"]').on('apply.daterangepicker', function(ev, picker) { + var project_id= $("#projects_filter").val(); + $('.user_summary').load('/daily_logs/user_logs',{ start_date: picker.startDate.format('YYYY-MM-DD'), end_date: picker.endDate.format('YYYY-MM-DD'),"project_id": project_id}); + }); + $('select').change(alert_me_test); + function alert_me_test(){ + var project_id= $("#projects_filter").val(); + $('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); + } + $('#context1 .menu .item') + .tab({ + context: $('#context1') + }) + ; + var today = new Date(); + $('#example1').calendar({ + onChange: function (date, text) { + var newValue = text; + $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); + }, + type: 'date', + maxDate: new Date(today.getFullYear(), today.getMonth(), today.getDate()) + }) + $('.top.menu .item').tab({'onVisible':function(){ + var newValue = $(this).attr("dataval") + $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); + }}); + $('.imageclass').on('click',function(){ + var userid=$(this).attr("id"); + $('.user_list').load('/daily_logs/user_logs',{"user_id":userid}); + }); - + $('.secondary.menu .item').tab({'onVisible':function(){ + var x=$(this).attr('data-tab'); + if(x=="first") + location.reload(); -$('.secondary.menu .item').tab({'onVisible':function(){ - var x=$(this).attr('data-tab'); - if(x=="first") - location.reload(); + }}) ; + }; -}}) ; -}; - diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index d3ef54f..0c8d220 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -12,15 +12,6 @@ body { margin-bottom: 60px; background-color: #f2f2f2; } -footer { - position: fixed; - bottom: 0; - width: 100%; - /* Set the fixed height of the footer here */ - height: 60px; - line-height: 60px; /* Vertically center the text there */ - background-color: #f5f5f5; -} /* Custom page CSS @@ -31,10 +22,6 @@ body > .container { padding: 60px 15px 0; } -.footer > .container { - padding-right: 15px; - padding-left: 15px; -} code { font-size: 80%; @@ -64,16 +51,31 @@ code { -moz-transition: all 0.5s ease; -o-transition: all 0.5s ease; transition: all 0.5s ease; + background-color:#394051; } .page { background-color:white; width : 50%; + padding-top:30px; +} +.centeralign +{ + padding-top:9px; } #wrapper.toggled #sidebar-wrapper { width: 250px; } +.headerclass +{ + padding-top:15px; + padding-bottom:5px; +} +.datecolor +{ + color:#55B6AE; +} #page-content-wrapper { width: 100%; @@ -88,7 +90,6 @@ code { } /* Sidebar Styles */ - .sidebar-nav { position: absolute; top: 0; @@ -106,18 +107,18 @@ code { .sidebar-nav li a { display: block; text-decoration: none; - color: #999999; + color:white ; } .sidebar-nav li a:hover { text-decoration: none; - color: #fff; + color: #f0f0f0; background: rgba(255,255,255,0.2); } .sidebar-nav li a:active, .sidebar-nav li a:focus { - color : #ffff; + color : #484848; } a.active { @@ -130,16 +131,16 @@ a.active } .sidebar-nav > .sidebar-brand a { - color: #999999; -} + color: white; + } .sidebar-nav > .sidebar-brand a:hover { color: #fff; background: none; } -input[type="text"] +.form-control { - width:50%; + width:100%; } #reportrange { @@ -149,6 +150,12 @@ input[type="text"] border: 1px solid #ccc; width: 100%; } +#datetime_ida +{ + border-color: #55B6AE; + color: #55b6ae; + background-color: white; +} .pull-right { text-align:right; @@ -156,8 +163,8 @@ input[type="text"] } .btn-default { - border: 2px solid #1aff1a; - color: #1aff1a; + border: 2px solid #45CDC5; + color: #45CDC5; background-color:white; } @@ -165,6 +172,13 @@ input[type="text"] { width:75%; } +.projects_filter +{ + width: 25%; + margin-bottom: 20px; + margin-top: 10px; + height: 35px; + } .imageclass { height: 150px ; @@ -175,4 +189,26 @@ input[type="text"] display: block; margin-left: auto; margin-right: auto; - } +} +.btn-change +{ + background-color:#55B6AE; + border-color:#55B6AE; + color:white; + margin-bottom:20px; + +} +.added +{ + width: 28%; + height: 40px; + background-color: white; + border-color: #E9E9E9; +} +.imagecircle +{ + border-radius: 50%; + width:50px; + height:50px; +} + diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index d6a558f..b211c9e 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -30,9 +30,10 @@ def new def create ActiveRecord::Base.transaction do - daily_log=DailyLog.new.create_log(current_user.id,params[:takeaway],params[:datetime_ida]) + daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) count = 0 count2=1 + byebug while count 'LogEntry',:dependent => :delete_all - belongs_to :user + has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all + belongs_to :user include ApplicationHelper - def create_log (user_id,takeaway,log_date) - #TODO save daily logs and its log entries in a active record transaction - ActiveRecord::Base.transaction do - DailyLog.create(log_date: log_date,user_id: user_id,takeaway: takeaway ) - end - - end - def user_create_summary(query_hash,project_id) - if(!project_id.nil?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("log_entries.project_id=#{project_id}") - else - daily_logs=DailyLog.where(query_hash) - end - log_summary=Hash.new - daily_logs.each_with_index do |daily_log,index| - summary_record=daily_log.create_record() - log_summary[index]=summary_record - end + #def create_log (user_id,takeaway,log_date) + ##TODO save daily logs and its log entries in a active record transaction + #ActiveRecord::Base.transaction do + #DailyLog.create(log_date: log_date,user_id: user_id,takeaway: takeaway ) + #end - log_summary +#end +def user_create_summary(query_hash,project_id) + if(!project_id.nil?) + daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("log_entries.project_id=#{project_id}") + else + daily_logs=DailyLog.where(query_hash) + end + log_summary=Hash.new + daily_logs.each_with_index do |daily_log,index| + summary_record=daily_log.create_record() + log_summary[index]=summary_record end - def create_summary(log_date:) - daily_logs= DailyLog.where(log_date: log_date) - log_summary=Hash.new - daily_logs.each_with_index do |daily_log,index| - summary_record = daily_log.create_record() - log_summary[index]=summary_record - end - log_summary + + log_summary +end + +def create_summary(log_date:) + daily_logs= DailyLog.where(log_date: log_date) + log_summary=Hash.new + daily_logs.each_with_index do |daily_log,index| + summary_record = daily_log.create_record() + log_summary[index]=summary_record end + log_summary +end + +def create_record() - def create_record() - - summary_record=Hash.new - summary_record[:name] = User.where("id=#{self.user_id}")[0].name; - #summary_record[:name] = current_user.name - #summary_record[:name]=User.find(key.user_id)[0].name - summary_record[:takeaway]=self.takeaway.to_s - summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; - summary_record[:beautifulcode]=self.company_record() - summary_record[:clients] =self.client_record() - summary_record[:logdate]=self.log_date - summary_record + summary_record=Hash.new + summary_record[:name] = User.where("id=#{self.user_id}")[0].name; + #summary_record[:name] = current_user.name + #summary_record[:name]=User.find(key.user_id)[0].name + summary_record[:takeaway]=self.takeaway.to_s + summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; + summary_record[:beautifulcode]=self.company_record() + summary_record[:clients] =self.client_record() + summary_record[:logdate]=self.log_date.strftime('%v') + summary_record +end +def company_record() + company_record={} + #TODO move where clauses to models instance methods + #if(project_id != nil) + #log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) + #else + log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) + #end + log_ids.each do |log_id| + company_record[LogEntry.find_by_id(log_id).project.name.to_s]=LogEntry.find_by_id(log_id).log_text.to_s; end - def company_record() - company_record={} - #TODO move where clauses to models instance methods - #if(project_id != nil) - #log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) + company_record +end +def client_record() + clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') + project_logs=Hash.new(); + clients.each do |client| + #if(project_id !=nil) + #project_log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) #else - log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) - #end - log_ids.each do |log_id| - company_record[LogEntry.find_by_id(log_id).project.name.to_s]=LogEntry.find_by_id(log_id).log_text.to_s; - end - company_record + project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + # end + project_log_logtext=Hash.new() + project_log_ids.each do |log_id,pid| + project_log_logtext[Project.find_by_id(pid).name]=LogEntry.find_by_id(log_id).log_text; + end + project_logs[client.to_s]=project_log_logtext; end - def client_record() - clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') - project_logs=Hash.new(); - clients.each do |client| - #if(project_id !=nil) - #project_log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) - #else - project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) - # end - project_log_logtext=Hash.new() - project_log_ids.each do |log_id,pid| - project_log_logtext[Project.find_by_id(pid).name]=LogEntry.find_by_id(log_id).log_text; - end - project_logs[client.to_s]=project_log_logtext; - end - project_logs + project_logs end end diff --git a/app/views/daily_logs/_user_list.html.haml b/app/views/daily_logs/_user_list.html.haml index cbba10c..4246390 100644 --- a/app/views/daily_logs/_user_list.html.haml +++ b/app/views/daily_logs/_user_list.html.haml @@ -10,4 +10,4 @@ =user[:name] %br :javascript - DailyLog(); \ No newline at end of file + DailyLog(); diff --git a/app/views/daily_logs/_user_summary.html.haml b/app/views/daily_logs/_user_summary.html.haml index 5c7eab7..8675989 100644 --- a/app/views/daily_logs/_user_summary.html.haml +++ b/app/views/daily_logs/_user_summary.html.haml @@ -5,7 +5,7 @@ .col-md-4 %h2 =value[:name] - .col-md-8.pull-right + .col-md-8.pull-right.datecolor =value[:logdate] %h3 Work @@ -33,4 +33,4 @@ =value[:takeaway] %br %br - %br \ No newline at end of file + %br diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index cd775c0..ec4e9b1 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -4,33 +4,36 @@ %input{ "type"=>"text","name"=>"daterange"} %div.row %div.col-md-4 - = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil} + = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil,:class => "projects_filter"} %div.row %div.col-md-8.page = form_tag daily_logs_path, :method => :post do .row + .col-md-1 + = image_tag (image_url(current_user.picture )), class:"imagecircle" .col-md-4 - %h2 + %h4.centeralign =current_user.name - .col-md-8.pull-right + .col-md-7.pull-right =date_field_tag(:datetime_ida, Date.today) - %h3 + %h3.headerclass Work %div.project - = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()" - %div.task - %br - %h3 - Task - %input.form-control{"maxlength"=>"300","name"=>"projects[]LogText[]","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} - %br + = select_tag "projects[id]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "Select project", :default => nil,:class => "added" + //%div.task + %br + %h5.task + Task %div + %input.form-control{"maxlength"=>"300","name"=>"projects[LogText][]","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} + %div + %br %button.btn.btn-default.add_field_button{"id"=>"company"} - add more - %hr + Add Task + %hr %div %button.btn.btn-default.add_project_button{} - add project + Add Project %hr %div %h3 @@ -46,7 +49,7 @@ what are your takeaways? %input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} %hr - = submit_tag("Submit", class: "btn btn-success btn-md btn-md pull-right" ) + = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) %br %br %br diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index a20148d..bb8b982 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -30,50 +30,26 @@ %script(src="//cdn.jsdelivr.net/bootstrap.daterangepicker/2/daterangepicker.js") %body - / Fixed navbar - / %nav.navbar.navbar-toggleable-md.navbar-inverse.fixed-top.bg-inverse - / %button.navbar-toggler.navbar-toggler-right{"aria-controls" => "navbarCollapse", "aria-expanded" => "false", "aria-label" => "Toggle navigation", "data-target" => "#navbarCollapse", "data-toggle" => "collapse", :type => "button"} - / %span.navbar-toggler-icon - / %a.navbar-brand{:href => "#"} DailyRecap - / #navbarCollapse.collapse.navbar-collapse - / %ul.navbar-nav.mr-auto - / %li.nav-item.active - / %a.nav-link{:href => "#"} - / Home - / %span.sr-only (current) - / %li.nav-item - / %a.nav-link{:href => "#"} Link - / %li.nav-item - / - if current_user - / = link_to 'Logout', signout_path, :class => "nav-link" - / - else - / = link_to 'Login', "/auth/google_oauth2", :class => "nav-link" - / %li.nav-item - / %a.nav-link.disabled{:href => "#"} Disabled - / %form.form-inline.mt-2.mt-md-0 - / %input.form-control.mr-sm-2{:placeholder => "Search", :type => "text"}/ - / %button.btn.btn-outline-success.my-2.my-sm-0{:type => "submit"} Search - / Begin page content - /.container %div.toggled{:id=>"wrapper"} %div{:id=>"sidebar-wrapper"} %ul.sidebar-nav %li.sidebar-brand %a{:href=>"#"} - Daily recap - + Daily Recap %li %a.active{:href=> new_daily_log_path} My Logs %li - %a{:href=> people_logs_path}> + %a{:href=> people_logs_path} People Logs %li - if current_user - = link_to 'Logout', signout_path, :class => "nav-link" + %a{:href => signout_path } + Logout - else - = link_to 'Login', "/auth/google_oauth2", :class => "nav-link" + %a{:href => "/auth/google_oauth2"} + Logout %div{:id=>"page-content-wrapper"} = yield From 2ab0833fbb607fff557012ddf074958fd7110a62 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 20 Jun 2017 14:23:48 +0530 Subject: [PATCH 02/17] edit_log_wip --- app/assets/javascripts/daily_log.js | 54 ++++---- app/assets/stylesheets/application.scss | 72 ++++++++++- app/controllers/daily_logs_controller.rb | 88 +++++++------ app/models/daily_log.rb | 125 +++++++++---------- app/views/daily_logs/_user_summary.html.haml | 16 ++- app/views/daily_logs/edit_log.html.haml | 85 +++++++++++++ app/views/daily_logs/new.html.haml | 18 ++- config/routes.rb | 5 +- 8 files changed, 324 insertions(+), 139 deletions(-) create mode 100644 app/views/daily_logs/edit_log.html.haml diff --git a/app/assets/javascripts/daily_log.js b/app/assets/javascripts/daily_log.js index c5fa361..ddd460f 100644 --- a/app/assets/javascripts/daily_log.js +++ b/app/assets/javascripts/daily_log.js @@ -1,27 +1,31 @@ - function DailyLog() { - $(document).delegate('.add_field_button', 'click', function() - { - $(this).parent().children().eq(1).append("
") - }); - $(document).delegate('.add_project_button', 'click', function() - { - $('.project').append("

Task


"); - var attrid="projects[project_id]"; - $('.added').attr('name', attrid).append($('#projects_project_id').html()); +function DailyLog() { + $(document).delegate('.add_field_button', 'click', function() + { + var project_id= $(this).parent().parent().children().eq(0).val(); + $(this).parent().children().eq(1).append("
"); + }); + $(document).delegate('.add_project_button', 'click', function() + { + $('.project').append("

Task

"); + var attrid="projects[]"; + $('.newproject').attr('name',attrid).append($('#projects_').html()); + $('.newproject').removeClass('newproject'); + //if ($('.newproject').html().length==0) + //$('.newproject').empty().append($('#projects_').html()); - }); + }); $(document).ready(function() { $('input[name="daterange"]').daterangepicker(); }); $('input[name="daterange"]').on('apply.daterangepicker', function(ev, picker) { var project_id= $("#projects_filter").val(); - $('.user_summary').load('/daily_logs/user_logs',{ start_date: picker.startDate.format('YYYY-MM-DD'), end_date: picker.endDate.format('YYYY-MM-DD'),"project_id": project_id}); + $('.user_summary').load('/daily_logs/user_logs',{ start_date: picker.startDate.format('YYYY-MM-DD'), end_date: picker.endDate.format('YYYY-MM-DD'),"project_id": project_id}); }); - $('select').change(alert_me_test); + $('.projects_filter').change(alert_me_test); function alert_me_test(){ var project_id= $("#projects_filter").val(); - $('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); + $('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); } $('#context1 .menu .item') .tab({ @@ -31,15 +35,15 @@ var today = new Date(); $('#example1').calendar({ onChange: function (date, text) { - var newValue = text; - $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); + var newValue = text; + $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); }, type: 'date', - maxDate: new Date(today.getFullYear(), today.getMonth(), today.getDate()) - }) + maxDate: new Date(today.getFullYear(), today.getMonth(), today.getDate()) + }) $('.top.menu .item').tab({'onVisible':function(){ - var newValue = $(this).attr("dataval") - $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); + var newValue = $(this).attr("dataval") + $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); }}); $('.imageclass').on('click',function(){ var userid=$(this).attr("id"); @@ -52,5 +56,13 @@ location.reload(); }}) ; - }; + // $('.added').change(function() { + $(document).delegate('.added', 'change', function() + { + var project_id =$(this).val(); + var name= 'Log_entry['+project_id+'][]'; + $(this).next().next().next().children().eq(1).children().filter('.form-control').attr('name',name); + $(this).next().next().next().children().filter('.form-control').attr('name',name); + }); +}; diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 0c8d220..8ba547f 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -37,7 +37,11 @@ code { #wrapper.toggled { padding-left: 250px; } - +.task +{ + padding-top:10px; + padding-bottom:5px +} #sidebar-wrapper { z-index: 1000; position: fixed; @@ -211,4 +215,68 @@ a.active width:50px; height:50px; } - +.btn { /* just for this demo. */ + margin-top: 5px; +} +.btn-arrow-right, +.btn-arrow-left { + position: relative; + padding-left: 18px; + padding-right: 18px; +} +.btn-arrow-right { + padding-left: 20px; +} +.btn-arrow-left { + padding-right: 20px; + height: 12%; +} +.btn-arrow-right:before, +.btn-arrow-right:after, +.btn-arrow-left:before, +.btn-arrow-left:after { /* make two squares (before and after), looking similar to the button */ + content:""; + position: absolute; + top: 5px; /* move it down because of rounded corners */ + width: 22px; /* same as height */ + height: 22px; /* button_outer_height / sqrt(2) */ + background: inherit; /* use parent background */ + border: inherit; /* use parent border */ + border-left-color: transparent; /* hide left border */ + border-bottom-color: transparent; /* hide bottom border */ + border-radius: 0px 4px 0px 0px; /* round arrow corner, the shorthand property doesn't accept "inherit" so it is set to 4px */ + -webkit-border-radius: 0px 4px 0px 0px; + -moz-border-radius: 0px 4px 0px 0px; +} +.btn-arrow-right:before, +.btn-arrow-right:after { + transform: rotate(45deg); /* rotate right arrow squares 45 deg to point right */ + -webkit-transform: rotate(45deg); + -moz-transform: rotate(45deg); + -o-transform: rotate(45deg); + -ms-transform: rotate(45deg); +} +.btn-arrow-left:before, +.btn-arrow-left:after { + transform: rotate(225deg); /* rotate left arrow squares 225 deg to point left */ + -webkit-transform: rotate(225deg); + -moz-transform: rotate(225deg); + -o-transform: rotate(225deg); + -ms-transform: rotate(225deg); +} +.btn-arrow-right:before, +.btn-arrow-left:before { /* align the "before" square to the left */ + left: -11px; +} +.btn-arrow-right:after, +.btn-arrow-left:after { /* align the "after" square to the right */ + right: -11px; +} +.btn-arrow-right:after, +.btn-arrow-left:before { /* bring arrow pointers to front */ + z-index: 0; +} +.btn-arrow-right:before, +.btn-arrow-left:after { /* hide arrow tails background */ + background-color: white; +} diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index b211c9e..208c344 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -2,19 +2,19 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token def new - #@projects=Project.where(client_name: Project::BEAUTIFUL_CODE_CLIENT) + #@projects=Project.where(client_name: Project::BEAUTIFUL_CODE_CLIENT) @daily_log =current_user.daily_logs.new @projects=Project.all if(!params[:start_date].nil?) - session[:start_date] =params[:start_date] - session[:end_date]=params[:end_date] + session[:start_date] =params[:start_date] + session[:end_date]=params[:end_date] end if(params[:project_id].nil? != true ) if(params[:project_id].empty?) - session[:project_id]=nil + session[:project_id]=nil else - session[:project_id] =params[:project_id] - end + session[:project_id] =params[:project_id] + end end query_hash = {} @@ -24,29 +24,37 @@ def new else query_hash.merge!(log_date: 1.week.ago..Date.today) end - @user_log_summary=@daily_log.user_create_summary(query_hash,session[:project_id]) - + @user_log_summary=@daily_log.user_create_summary(query_hash,session[:project_id]) + end def create ActiveRecord::Base.transaction do - daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) - count = 0 - count2=1 - byebug - while count "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} + @log_summary=DailyLog.new.create_summary(log_date: params[:log_date]) + render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} end def user_logs - query_hash = {} - if(!params[:user_id].nil?) + query_hash = {} + if(!params[:user_id].nil?) query_hash.merge!(user_id: params[:user_id]) - else + else query_hash.merge!(user_id: current_user.id) end if(!params[:start_date].nil?) query_hash.merge!(log_date: params[:start_date]..params[:end_date]) else - query_hash.merge!(log_date: 1.week.ago..Date.today) - end - if(params[:project_id].nil? or params[:project_id].empty?) - @user_logs=DailyLog.new.user_create_summary(query_hash,nil) - else - @user_logs=DailyLog.new.user_create_summary(query_hash,params[:project_id]) - end + query_hash.merge!(log_date: 1.week.ago..Date.today) + end + if(params[:project_id].nil? or params[:project_id].empty?) + @user_logs=DailyLog.new.user_create_summary(query_hash,nil) + else + @user_logs=DailyLog.new.user_create_summary(query_hash,params[:project_id]) + end render :partial => "daily_logs/user_summary",object: @user_logs,locals: { log_summary: @user_logs} end + def edit_log + @edit_log = DailyLog.find(params[:log_id]).create_record(); + @projects=Project.all + end end diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index b7d5f59..7b46476 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -2,80 +2,73 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all belongs_to :user include ApplicationHelper - #def create_log (user_id,takeaway,log_date) - ##TODO save daily logs and its log entries in a active record transaction - #ActiveRecord::Base.transaction do - #DailyLog.create(log_date: log_date,user_id: user_id,takeaway: takeaway ) - #end - -#end -def user_create_summary(query_hash,project_id) - if(!project_id.nil?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("log_entries.project_id=#{project_id}") - else - daily_logs=DailyLog.where(query_hash) - end - log_summary=Hash.new - daily_logs.each_with_index do |daily_log,index| - summary_record=daily_log.create_record() - log_summary[index]=summary_record - end + def user_create_summary(query_hash,project_id) + if(!project_id.nil?) + daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("log_entries.project_id=#{project_id}") + else + daily_logs=DailyLog.where(query_hash) + end + log_summary=Hash.new + daily_logs.each_with_index do |daily_log,index| + summary_record=daily_log.create_record() + log_summary[index]=summary_record + end - log_summary -end + log_summary + end -def create_summary(log_date:) - daily_logs= DailyLog.where(log_date: log_date) - log_summary=Hash.new - daily_logs.each_with_index do |daily_log,index| - summary_record = daily_log.create_record() - log_summary[index]=summary_record + def create_summary(log_date:) + daily_logs= DailyLog.where(log_date: log_date) + log_summary=Hash.new + daily_logs.each_with_index do |daily_log,index| + summary_record = daily_log.create_record() + log_summary[index]=summary_record + end + log_summary end - log_summary -end -def create_record() + def create_record() - summary_record=Hash.new - summary_record[:name] = User.where("id=#{self.user_id}")[0].name; - #summary_record[:name] = current_user.name - #summary_record[:name]=User.find(key.user_id)[0].name - summary_record[:takeaway]=self.takeaway.to_s - summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; - summary_record[:beautifulcode]=self.company_record() - summary_record[:clients] =self.client_record() - summary_record[:logdate]=self.log_date.strftime('%v') - summary_record -end -def company_record() - company_record={} - #TODO move where clauses to models instance methods - #if(project_id != nil) - #log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) - #else - log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) - #end - log_ids.each do |log_id| - company_record[LogEntry.find_by_id(log_id).project.name.to_s]=LogEntry.find_by_id(log_id).log_text.to_s; + summary_record=Hash.new + summary_record[:name] = User.where("id=#{self.user_id}")[0].name; + summary_record[:picture]= User.where("id=#{self.user_id}")[0].picture; + summary_record[:log_id] = self.id; + summary_record[:takeaway]=self.takeaway.to_s + summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; + summary_record[:beautifulcode]=self.company_record() + summary_record[:clients] =self.client_record() + summary_record[:logdate]=self.log_date.strftime('%v') + summary_record end - company_record -end -def client_record() - clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') - project_logs=Hash.new(); - clients.each do |client| - #if(project_id !=nil) - #project_log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + def company_record() + company_record={} + #TODO move where clauses to models instance methods + #if(project_id != nil) + #log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) #else - project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) - # end - project_log_logtext=Hash.new() - project_log_ids.each do |log_id,pid| - project_log_logtext[Project.find_by_id(pid).name]=LogEntry.find_by_id(log_id).log_text; + log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) + #end + log_ids.each do |log_id| + company_record[LogEntry.find_by_id(log_id).project.name.to_s]=LogEntry.find_by_id(log_id).log_text.to_s; end - project_logs[client.to_s]=project_log_logtext; + company_record + end + def client_record() + clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') + project_logs=Hash.new(); + clients.each do |client| + #if(project_id !=nil) + #project_log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + #else + project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + # end + project_log_logtext=Hash.new() + project_log_ids.each do |log_id,pid| + project_log_logtext[Project.find_by_id(pid).name]=LogEntry.find_by_id(log_id).log_text; + end + project_logs[client.to_s]=project_log_logtext; + end + project_logs end - project_logs -end end diff --git a/app/views/daily_logs/_user_summary.html.haml b/app/views/daily_logs/_user_summary.html.haml index 8675989..046f9b5 100644 --- a/app/views/daily_logs/_user_summary.html.haml +++ b/app/views/daily_logs/_user_summary.html.haml @@ -2,7 +2,9 @@ %div.row %div.col-md-8.page .row - .col-md-4 + .col-md-1 + = image_tag (image_url(value[:picture] )), class:"imagecircle" + .col-md-3 %h2 =value[:name] .col-md-8.pull-right.datecolor @@ -10,27 +12,29 @@ %h3 Work -value[:beautifulcode].each do |project,logtext| - %div + %button.btn.btn-default.btn-arrow-left =project %p =logtext -value[:clients].each do |client,records| - %h5 - =client -records.each do |pname,log| - %div + %a.ui.tag.label =pname %p =log + %hr %h5 Learning %p =value[:learning] + %hr %h5 TakeAway - %p + %div{display:"list-item"} =value[:takeaway] + %div + =link_to "Edit log",edit_log_path(:log_id => value[:log_id]) %br %br %br diff --git a/app/views/daily_logs/edit_log.html.haml b/app/views/daily_logs/edit_log.html.haml new file mode 100644 index 0000000..7dcb7f9 --- /dev/null +++ b/app/views/daily_logs/edit_log.html.haml @@ -0,0 +1,85 @@ +%div.row + %div.col-md-8.page + .row + .col-md-4 + %h2 + =@edit_log[:name] + .col-md-8.pull-right.datecolor + =@edit_log[:logdate] + %h3 + Work + -@edit_log[:beautifulcode].each do |project,logtext| + %div + =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :default => nil,:class => "added" + + %p + =logtext + -@edit_log[:clients].each do |client,records| + %h5 + =client + -records.each do |project,logtext| + %div + =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :default => nil,:class => "added" + %p + =text_field_tag("projects[]Logtext[]","#{logtext}") + + %h5 + Learning + %p + =text_field_tag("LearningLog","#{@edit_log[:learning]}") + %h5 + TakeAway + %p + =text_field_tag("takeaway","#{@edit_log[:takeaway]}") + %br + %br + %br +-#%div.row + -#%div.col-md-8.page + -#= form_tag daily_logs_path, :method => :post do + -#.row + -#.col-md-1 + -#= image_tag (image_url(current_user.picture )), class:"imagecircle" + -#.col-md-4 + -#%h4.centeralign + -#=@edit_log[:name] + -#.col-md-7.pull-right + -#=date_field_tag(:datetime_ida, @edit_log[:logdate]) + -#%h3.headerclass + -#Work + -#%div.project + -#=select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "Select project", :default => nil,:class => "added" + -#//%div.task + -#%br + -#%h5.task + -#Task + -#%div + -#%input.form-control{"maxlength"=>"300","name"=>"Logs[][LogText][]","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} + -#%div + -#%br + -#%button.btn.btn-default.add_field_button{"id"=>"company"} + -#Add Task + -#%hr + -#%div + -#%button.btn.btn-default.add_project_button{} + -#Add Project + -#%hr + -#%div + -#%h3 + -#Learning + -#%p + -#what did you learn today? + -#%input.form-control{"name"=>"LearningLog","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} + -#%hr + -#%div + -#%h3 + -#Takeaways + -#%p + -#what are your takeaways? + -#%input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} + -#%hr + -#= submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) +-#%br +-#%br +-#%br + diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index ec4e9b1..d306744 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -1,7 +1,14 @@ %div.container-fluid %div.row %div.col-md-4 - %input{ "type"=>"text","name"=>"daterange"} + -#%input{ "type"=>"text","name"=>"daterange"} + -#%i.fa.fa-calendar.form-control-feedback + #calendar2.ui + .ui.input.left.icon + %i.calendar.icon + %input{ "type"=>"text","name"=>"daterange"} + + %div.row %div.col-md-4 = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil,:class => "projects_filter"} @@ -19,13 +26,13 @@ %h3.headerclass Work %div.project - = select_tag "projects[id]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "Select project", :default => nil,:class => "added" + = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:class => "added",:prompt => "Select Project", :required => "required" //%div.task %br %h5.task Task %div - %input.form-control{"maxlength"=>"300","name"=>"projects[LogText][]","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} + %input.form-control{"maxlength"=>"300","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} %div %br %button.btn.btn-default.add_field_button{"id"=>"company"} @@ -40,7 +47,7 @@ Learning %p what did you learn today? - %input.form-control{"name"=>"LearningLog","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} + %input.form-control{"name"=>"learning_log","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} %hr %div %h3 @@ -61,3 +68,6 @@ :javascript DailyLog(); + $(document).ready(function() { + + }); diff --git a/config/routes.rb b/config/routes.rb index c3872be..67dbde6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Rails.application.routes.draw do - get 'auth/:provider/callback', to: 'sessions#create' + get 'auth/:provider/callback', to: 'sessions#create' get 'auth/failure', to: redirect('/') get 'signout', to: 'sessions#destroy', as: 'signout' get '/summary', :to => 'daily_logs#summary', as: 'summary' @@ -10,6 +10,7 @@ post 'daily_logs/refresh', :to => 'daily_logs#refresh' get 'daily_logs/user_list', :to => 'daily_logs#user_list' match 'daily_logs/user_logs', :to => 'daily_logs#user_logs', via:[:GET,:POST] + match 'daily_logs/edit_log', :to => 'daily_logs#edit_log', via:[:GET,:POST],as:'edit_log' resources :sessions, only: [:create, :destroy] resources :daily_logs, only: [:new, :create] @@ -17,6 +18,6 @@ # get "/daily_logs/new", on: :collection # end resources :projects - + root :to => 'daily_logs#index' end From 926a21e91531a68082c5691ea6fcd7c30d3323d9 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Thu, 22 Jun 2017 14:18:57 +0530 Subject: [PATCH 03/17] edit_log completed --- app/assets/javascripts/application.js | 1 + app/assets/javascripts/daily_log.js | 17 ++- app/assets/javascripts/edit_log.js | 37 +++++ app/controllers/daily_logs_controller.rb | 116 ++++++++-------- app/helpers/daily_logs_helper.rb | 17 ++- app/models/daily_log.rb | 46 ++++--- app/views/daily_logs/_user_summary.html.haml | 23 ++-- app/views/daily_logs/edit_log.html.haml | 136 ++++++++----------- app/views/daily_logs/index.html.haml | 1 - app/views/daily_logs/new.html.haml | 91 ++++++------- app/views/layouts/application.html.haml | 6 +- config/routes.rb | 1 + spec/models/feedback_spec.rb | 11 +- spec/models/logentry_spec.rb | 16 +-- spec/models/project_spec.rb | 53 +++----- spec/models/user_spec.rb | 101 ++++++-------- 16 files changed, 345 insertions(+), 328 deletions(-) create mode 100644 app/assets/javascripts/edit_log.js diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 1a1f2b0..a2d6ea1 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -16,5 +16,6 @@ //= require bootstrap-sprockets //= require cocoon //= require daily_log +//= require edit_log diff --git a/app/assets/javascripts/daily_log.js b/app/assets/javascripts/daily_log.js index ddd460f..946dfbe 100644 --- a/app/assets/javascripts/daily_log.js +++ b/app/assets/javascripts/daily_log.js @@ -10,13 +10,13 @@ function DailyLog() { var attrid="projects[]"; $('.newproject').attr('name',attrid).append($('#projects_').html()); $('.newproject').removeClass('newproject'); - //if ($('.newproject').html().length==0) - //$('.newproject').empty().append($('#projects_').html()); - }); $(document).ready(function() { $('input[name="daterange"]').daterangepicker(); + //$('#datetime_ida').datepicker({ + //endDate: "+0d"; + //}) }); $('input[name="daterange"]').on('apply.daterangepicker', function(ev, picker) { var project_id= $("#projects_filter").val(); @@ -25,7 +25,16 @@ function DailyLog() { $('.projects_filter').change(alert_me_test); function alert_me_test(){ var project_id= $("#projects_filter").val(); - $('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); + var datepicker = $('input[name="daterange"]').data('daterangepicker'); + var today=moment().format('DD-MMM-YYYY'); + if(datepicker.startDate.format('DD-MMM-YYYY')==today) + { + $('.user_summary').load('/daily_logs/user_logs',{"project_id": project_id}); + } + else + { + $('.user_summary').load('/daily_logs/user_logs',{start_date:datepicker.startDate.format('YYYY-MM-DD'),end_date:datepicker.endDate.format('YYYY-MM-DD'),"project_id": project_id}); + } } $('#context1 .menu .item') .tab({ diff --git a/app/assets/javascripts/edit_log.js b/app/assets/javascripts/edit_log.js new file mode 100644 index 0000000..bc0b7d1 --- /dev/null +++ b/app/assets/javascripts/edit_log.js @@ -0,0 +1,37 @@ +function EditLog() { + $(document).delegate('.add_field_button', 'click', function() + { + var project_id= $(this).parent().parent().children().eq(0).children().val(); + if(project_id=="") + { + var project_id= $(this).parent().parent().children().eq(0).children().attr("value"); + } + + $(this).parent().children().eq(0).append("
"); + }); + $(document).delegate('.added', 'change', function() + { + var project_id =$(this).val(); + if(project_id=="") + { + project_id= $(this).attr("value"); + } + var name= 'NewLogtext['+project_id+'][]'; + $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.NewlyAdded').attr('name',name); + $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.existing').each(function(i){ + var oldName= $(this).attr('name'); + var newName='Logtext['+project_id+']'+oldName.substring(oldName.lastIndexOf('[')); + $(this).attr('name',newName); + }); + //$(this).next().next().next().children().filter('.form-control').attr('name',name); + }); + $(document).delegate('.add_project_button','click',function() + { + var value= $('#projects_').attr('value'); + var attrid ="projects[]" + $('.project').append("
Task

"); + $('.newproject').attr('name',attrid).append($('#projects_').html()); + $('.newproject').removeClass('newproject') + }); +}; + diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index 208c344..002bf2a 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -2,61 +2,62 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token def new - #@projects=Project.where(client_name: Project::BEAUTIFUL_CODE_CLIENT) - @daily_log =current_user.daily_logs.new - @projects=Project.all - if(!params[:start_date].nil?) - session[:start_date] =params[:start_date] - session[:end_date]=params[:end_date] - end - if(params[:project_id].nil? != true ) - if(params[:project_id].empty?) - session[:project_id]=nil - else - session[:project_id] =params[:project_id] - end - end + if(current_user.nil?) + url="/auth/google_oauth2" + redirect_to(url) + else + #@projects=Project.where(client_name: Project::BEAUTIFUL_CODE_CLIENT) + @daily_log =current_user.daily_logs.new + @projects=Project.all + #if(!params[:start_date].nil?) + #session[:start_date] =params[:start_date] + #session[:end_date]=params[:end_date] + #end + #if(params[:project_id].nil? != true ) + #if(params[:project_id].empty?) + #session[:project_id]=nil + #else + #session[:project_id] =params[:project_id] + #end + #end - query_hash = {} - query_hash.merge!(user_id: current_user.id) - if(session[:start_date] != nil) - query_hash.merge!(log_date: session[:start_date]..session[:end_date]) + query_hash = build_query_hash(params) + # query_hash.merge!(user_id: current_user.id) + #if(session[:start_date] != nil) + #query_hash.merge!(log_date: session[:start_date]..session[:end_date]) + #else + #query_hash.merge!(log_date: 1.week.ago..Date.today) + #end + #@user_log_summary=@daily_log.user_create_summary(query_hash,session[:project_id]) + if(params[:project_id].nil? or params[:project_id].empty?) + @user_log_summary=DailyLog.new.user_create_summary(query_hash,nil) else - query_hash.merge!(log_date: 1.week.ago..Date.today) + @user_log_summary=DailyLog.new.user_create_summary(query_hash,params[:project_id]) + end + end - @user_log_summary=@daily_log.user_create_summary(query_hash,session[:project_id]) end def create - ActiveRecord::Base.transaction do - daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) - #count = 0 - #count2=1 - #byebug - #while count "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} end def user_logs - query_hash = {} - if(!params[:user_id].nil?) - query_hash.merge!(user_id: params[:user_id]) - else - query_hash.merge!(user_id: current_user.id) - end - if(!params[:start_date].nil?) - query_hash.merge!(log_date: params[:start_date]..params[:end_date]) - else - query_hash.merge!(log_date: 1.week.ago..Date.today) - end + query_hash = build_query_hash(params) if(params[:project_id].nil? or params[:project_id].empty?) @user_logs=DailyLog.new.user_create_summary(query_hash,nil) else @user_logs=DailyLog.new.user_create_summary(query_hash,params[:project_id]) end - render :partial => "daily_logs/user_summary",object: @user_logs,locals: { log_summary: @user_logs} end def edit_log @edit_log = DailyLog.find(params[:log_id]).create_record(); @projects=Project.all + @daily_log_id=params[:log_id] + + end + def finish_edit + DailyLog.find_by_id(params[:daily_log_id]).update_records(params[:Logtext],params[:Learning_log],params[:takeaway]) + if(!params[:NewLogtext].nil?) + params[:NewLogtext].each do |project_id,log_entries| + log_entries.each do|logtext| + DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) + end + end + end + redirect_to new_daily_log_path end end + diff --git a/app/helpers/daily_logs_helper.rb b/app/helpers/daily_logs_helper.rb index c648eb7..127ab10 100644 --- a/app/helpers/daily_logs_helper.rb +++ b/app/helpers/daily_logs_helper.rb @@ -1,3 +1,18 @@ module DailyLogsHelper + def build_query_hash(params) + query_hash ={} + if(!params[:user_id].nil?) + query_hash.merge!(user_id: params[:user_id]) + else + query_hash.merge!(user_id: current_user.id) + end + if(!params[:start_date].nil?) + query_hash.merge!(log_date: params[:start_date]..params[:end_date]) + else + query_hash.merge!(log_date: 1.week.ago..Date.today) + end + query_hash + + end +end -end \ No newline at end of file diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 7b46476..ee84adb 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -2,24 +2,24 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all belongs_to :user include ApplicationHelper - def user_create_summary(query_hash,project_id) + def user_create_summary(query_hash,project_id)# if(!project_id.nil?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("log_entries.project_id=#{project_id}") + daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("project_id=#{project_id}").order("log_date DESC").distinct + #daily_logs =DailyLog.joins(:log_entries).where(query_hash).where("log_entries.project_id=#{project_id}").order("log_date DESC").distinct else - daily_logs=DailyLog.where(query_hash) + daily_logs=DailyLog.where(query_hash).order("log_date DESC") + end log_summary=Hash.new daily_logs.each_with_index do |daily_log,index| summary_record=daily_log.create_record() log_summary[index]=summary_record end - - log_summary end def create_summary(log_date:) - daily_logs= DailyLog.where(log_date: log_date) + daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") log_summary=Hash.new daily_logs.each_with_index do |daily_log,index| summary_record = daily_log.create_record() @@ -29,9 +29,9 @@ def create_summary(log_date:) end def create_record() - summary_record=Hash.new summary_record[:name] = User.where("id=#{self.user_id}")[0].name; + summary_record[:user_id]= User.where("id=#{self.user_id}")[0].id; summary_record[:picture]= User.where("id=#{self.user_id}")[0].picture; summary_record[:log_id] = self.id; summary_record[:takeaway]=self.takeaway.to_s @@ -44,31 +44,33 @@ def create_record() def company_record() company_record={} #TODO move where clauses to models instance methods - #if(project_id != nil) - #log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) - #else - log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id) - #end - log_ids.each do |log_id| - company_record[LogEntry.find_by_id(log_id).project.name.to_s]=LogEntry.find_by_id(log_id).log_text.to_s; + project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id,:project_id) + project_log_logtext = Hash.new{|h,k| h[k] = []} + project_log_ids.each do |x| + project_log_logtext[Project.find_by_id(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] end - company_record + project_log_logtext end def client_record() clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') project_logs=Hash.new(); clients.each do |client| - #if(project_id !=nil) - #project_log_ids=LogEntry.where(:daily_log_id => self.id,:project_id => project_id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) - #else project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) - # end - project_log_logtext=Hash.new() - project_log_ids.each do |log_id,pid| - project_log_logtext[Project.find_by_id(pid).name]=LogEntry.find_by_id(log_id).log_text; + project_log_logtext = Hash.new{|h,k| h[k] = []} + project_log_ids.each do |x| + project_log_logtext[Project.find_by_id(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] end project_logs[client.to_s]=project_log_logtext; end project_logs end + def update_records(records,learning,takeaway) + records.each do |project_id,log_entries| + log_entries.each do|logentry_id,log_text| + self.log_entries.find_by_id(logentry_id).update_attributes(project_id: project_id,log_text:log_text); + end + end + self.update_attributes(:takeaway => takeaway) + self.log_entries.find_by_project_id(4).update_attributes(log_text: learning) + end end diff --git a/app/views/daily_logs/_user_summary.html.haml b/app/views/daily_logs/_user_summary.html.haml index 046f9b5..feab829 100644 --- a/app/views/daily_logs/_user_summary.html.haml +++ b/app/views/daily_logs/_user_summary.html.haml @@ -11,17 +11,21 @@ =value[:logdate] %h3 Work - -value[:beautifulcode].each do |project,logtext| - %button.btn.btn-default.btn-arrow-left + -value[:beautifulcode].each do |project,logtexts| + %a.ui.tag.label =project - %p - =logtext + %ul + -logtexts.each do |logtext| + %li + =logtext[0] -value[:clients].each do |client,records| - -records.each do |pname,log| + -records.each do |pname,logs| %a.ui.tag.label =pname - %p - =log + %ul + -logs.each do|log| + %li + =log[0] %hr %h5 @@ -33,8 +37,9 @@ TakeAway %div{display:"list-item"} =value[:takeaway] - %div - =link_to "Edit log",edit_log_path(:log_id => value[:log_id]) + -if(current_user.id == value[:user_id]) + %div + =link_to "Edit log",edit_log_path(:log_id => value[:log_id]),:class => "btn btn-change btn-md pull-right" %br %br %br diff --git a/app/views/daily_logs/edit_log.html.haml b/app/views/daily_logs/edit_log.html.haml index 7dcb7f9..2bb5f98 100644 --- a/app/views/daily_logs/edit_log.html.haml +++ b/app/views/daily_logs/edit_log.html.haml @@ -1,85 +1,67 @@ -%div.row - %div.col-md-8.page - .row - .col-md-4 - %h2 - =@edit_log[:name] - .col-md-8.pull-right.datecolor - =@edit_log[:logdate] - %h3 - Work - -@edit_log[:beautifulcode].each do |project,logtext| - %div - =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :default => nil,:class => "added" +%div.user_summary + %div.row + %div.col-md-8.page + =form_tag finish_edit_path, :method => :post do + .row + .col-md-1 + = image_tag (image_url(@edit_log[:picture] )), class:"imagecircle" + .col-md-4 + %h2 + =@edit_log[:name] + .col-md-7.pull-right.datecolor + =@edit_log[:logdate] + %h3.headerclass + Work + -@edit_log[:beautifulcode].each do |project,logtexts| + %div + %div + =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :value => @projects.where(:name =>"#{project}").pluck(:id).first.to_s,:class => "added" + %h5.task + Task + %div + %div + -logtexts.each do |logtext| + =text_field_tag("Logtext["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") + %br + %button.btn.btn-default.add_field_button{"id"=>"company"} + Add Task + %br + %div.project + -@edit_log[:clients].each do |client,records| + -records.each do |project,logtexts| + %div + %div + =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :value => @projects.where(:name =>"#{project}").pluck(:id).first.to_s,:class => "added" + %h5.task + Task + %div + %div + -logtexts.each do |logtext| + =text_field_tag("Logtext["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") + %br + %button.btn.btn-default.add_field_button{"id"=>"company"} + Add Task + %hr + %button.btn.btn-default.add_project_button + Add Project + %br + %h5 + Learning %p - =logtext - -@edit_log[:clients].each do |client,records| + =text_field_tag("Learning_log","#{@edit_log[:learning]}",:class => "form-control") %h5 - =client - -records.each do |project,logtext| - %div - =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :default => nil,:class => "added" - %p - =text_field_tag("projects[]Logtext[]","#{logtext}") + TakeAway + %p + =text_field_tag("takeaway","#{@edit_log[:takeaway]}",class: "form-control") + %input{"type" => "hidden","value" => "#{@daily_log_id}","name" => "daily_log_id"} + %hr + = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) - %h5 - Learning - %p - =text_field_tag("LearningLog","#{@edit_log[:learning]}") - %h5 - TakeAway - %p - =text_field_tag("takeaway","#{@edit_log[:takeaway]}") %br %br %br --#%div.row - -#%div.col-md-8.page - -#= form_tag daily_logs_path, :method => :post do - -#.row - -#.col-md-1 - -#= image_tag (image_url(current_user.picture )), class:"imagecircle" - -#.col-md-4 - -#%h4.centeralign - -#=@edit_log[:name] - -#.col-md-7.pull-right - -#=date_field_tag(:datetime_ida, @edit_log[:logdate]) - -#%h3.headerclass - -#Work - -#%div.project - -#=select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "Select project", :default => nil,:class => "added" - -#//%div.task - -#%br - -#%h5.task - -#Task - -#%div - -#%input.form-control{"maxlength"=>"300","name"=>"Logs[][LogText][]","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} - -#%div - -#%br - -#%button.btn.btn-default.add_field_button{"id"=>"company"} - -#Add Task - -#%hr - -#%div - -#%button.btn.btn-default.add_project_button{} - -#Add Project - -#%hr - -#%div - -#%h3 - -#Learning - -#%p - -#what did you learn today? - -#%input.form-control{"name"=>"LearningLog","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} - -#%hr - -#%div - -#%h3 - -#Takeaways - -#%p - -#what are your takeaways? - -#%input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} - -#%hr - -#= submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) --#%br --#%br --#%br + +:javascript + EditLog(); diff --git a/app/views/daily_logs/index.html.haml b/app/views/daily_logs/index.html.haml index f816517..f364cf7 100644 --- a/app/views/daily_logs/index.html.haml +++ b/app/views/daily_logs/index.html.haml @@ -1,5 +1,4 @@ %pre - Find me at `app/views/home/index.html.haml` - if current_user = link_to 'fill the daily log', new_daily_log_path, class: 'btn btn-primary btn-lg' - else diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index d306744..749ef83 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -7,59 +7,57 @@ .ui.input.left.icon %i.calendar.icon %input{ "type"=>"text","name"=>"daterange"} - - %div.row %div.col-md-4 = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil,:class => "projects_filter"} -%div.row - %div.col-md-8.page - = form_tag daily_logs_path, :method => :post do - .row - .col-md-1 - = image_tag (image_url(current_user.picture )), class:"imagecircle" - .col-md-4 - %h4.centeralign - =current_user.name - .col-md-7.pull-right - =date_field_tag(:datetime_ida, Date.today) - %h3.headerclass - Work - %div.project - = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:class => "added",:prompt => "Select Project", :required => "required" - //%div.task - %br - %h5.task - Task - %div - %input.form-control{"maxlength"=>"300","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} - %div +-if(@user_log_summary[0][:logdate] !=Date.today().strftime("%v")) + %div.row + %div.col-md-8.page + = form_tag daily_logs_path, :method => :post do + .row + .col-md-1 + = image_tag (image_url(current_user.picture )), class:"imagecircle" + .col-md-4 + %h4.centeralign + =current_user.name + .col-md-7.pull-right + =date_field_tag(:datetime_ida, Date.today) + %h3.headerclass + Work + %div.project + = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:class => "added",:prompt => "Select Project", :required => "required" %br - %button.btn.btn-default.add_field_button{"id"=>"company"} - Add Task - %hr - %div - %button.btn.btn-default.add_project_button{} - Add Project - %hr - %div - %h3 - Learning - %p - what did you learn today? - %input.form-control{"name"=>"learning_log","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} + %h5.task + Task + %div + %input.form-control{"maxlength"=>"300","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} + %div + %br + %button.btn.btn-default.add_field_button{"id"=>"company"} + Add Task %hr %div - %h3 - Takeaways - %p - what are your takeaways? - %input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} + %button.btn.btn-default.add_project_button{} + Add Project %hr - = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) -%br -%br -%br + %div + %h3 + Learning + %p + what did you learn today? + %input.form-control{"name"=>"learning_log","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} + %hr + %div + %h3 + Takeaways + %p + what are your takeaways? + %input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} + %hr + = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) + %br + %br + %br .user_summary =render :partial => "daily_logs/user_summary",object: @user_log_summary,locals: { log_summary: @user_log_summary} @@ -69,5 +67,4 @@ :javascript DailyLog(); $(document).ready(function() { - }); diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index bb8b982..5d04f01 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -49,7 +49,11 @@ Logout - else %a{:href => "/auth/google_oauth2"} - Logout + Login + #container.container + - flash.each do |message_type, message| + .alert{ class: "alert-#{message_type}" } + = message %div{:id=>"page-content-wrapper"} = yield diff --git a/config/routes.rb b/config/routes.rb index 67dbde6..f876c36 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,6 +11,7 @@ get 'daily_logs/user_list', :to => 'daily_logs#user_list' match 'daily_logs/user_logs', :to => 'daily_logs#user_logs', via:[:GET,:POST] match 'daily_logs/edit_log', :to => 'daily_logs#edit_log', via:[:GET,:POST],as:'edit_log' + match 'daily_logs/finish_edit', :to =>'daily_logs#finish_edit',via: [:POST], as:'finish_edit' resources :sessions, only: [:create, :destroy] resources :daily_logs, only: [:new, :create] diff --git a/spec/models/feedback_spec.rb b/spec/models/feedback_spec.rb index b7bc18f..9b24bf1 100644 --- a/spec/models/feedback_spec.rb +++ b/spec/models/feedback_spec.rb @@ -7,15 +7,12 @@ expect(feedbackentry).to respond_to(msg) end end -<<<<<<< HEAD it "should belongs to user" do - t = Feedback.reflect_on_association(:user) - t.macro.should == :belongs_to + association = Feedback.reflect_on_association(:user) + association.macro.should == :belongs_to end it "should belongs to daily_log" do - t = Feedback.reflect_on_association(:daily_log) - t.macro.should == :belongs_to + association = Feedback.reflect_on_association(:daily_log) + association.macro.should == :belongs_to end -======= ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 end diff --git a/spec/models/logentry_spec.rb b/spec/models/logentry_spec.rb index 84f65d1..323140f 100644 --- a/spec/models/logentry_spec.rb +++ b/spec/models/logentry_spec.rb @@ -1,22 +1,20 @@ require 'rails_helper' RSpec.describe LogEntry, type: :model do - let(:entry) { FactoryGirl.build(:log_entry) } - [:log_text, :project_id,:daily_log_id].each do |msg| + let(:entry) { FactoryGirl.build(:log_entry) } + [:log_text, :project_id,:daily_log_id].each do |msg| it "should respond to #{msg}" do expect(entry).to respond_to(msg) end end -<<<<<<< HEAD + it "should belongs to daily_log" do - t = LogEntry.reflect_on_association(:daily_log) - t.macro.should == :belongs_to + association = LogEntry.reflect_on_association(:daily_log) + association.macro.should == :belongs_to end it "should belongs to project" do - t = LogEntry.reflect_on_association(:project) - t.macro.should == :belongs_to + association = LogEntry.reflect_on_association(:project) + association.macro.should == :belongs_to end -======= ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a061d0d..f8c3425 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,51 +1,38 @@ require 'rails_helper' RSpec.describe Project, type: :model do -<<<<<<< HEAD - let(:project) { FactoryGirl.build(:Project) } - let(:id) { '111111' } - [:name, :client_name].each do |msg| -======= + #<<<<<<< HEAD + #let(:project) { FactoryGirl.build(:Project) } + #let(:id) { '111111' } + #[:name, :client_name].each do |msg| + #======= let(:project) { FactoryGirl.build(:project) } - [:name, :client_name].each do |msg| ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 it "should respond to #{msg}" do expect(project).to respond_to(msg) end end -<<<<<<< HEAD + it "should have many users " do - t = Project.reflect_on_association(:users) - t.macro.should == :has_and_belongs_to_many + association = Project.reflect_on_association(:users) + association.macro.should == :has_and_belongs_to_many end it "should have many log_entries" do - t = Project.reflect_on_association(:log_entries) - t.macro.should == :has_many + association = Project.reflect_on_association(:log_entries) + association.macro.should == :has_many end - context 'when newly created' do - it 'should not have any log entries' do - LogEntry.joins(:project).where("projects.id=1111").count.should == 0 - end - it 'should not have any users' do - User.joins(:project).where("projects.id=1111").count.should == 0 - - end - end - + #context 'when newly created' do + #it 'should not have any log entries' do + #LogEntry.joins(:project).where("projects.id=1111").count.should == 0 + #end + #it 'should not have any users' do + #User.joins(:project).where("projects.id=1111").count.should == 0 + #end + #end - #it 'should not have any users' - end -======= - it 'should have many users' + #it 'should not have any users' +end - it 'should have many log entries' - context 'when newly created' do - it 'should not have any log entries' - it 'should not have any users' - end -end ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4463a63..2a01315 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,82 +2,61 @@ RSpec.describe User, type: :model do - describe :from_omniauth do -<<<<<<< HEAD -======= - ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 + describe :from_omniauth do it 'should get user details from oauth after successful authentication' do - OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new({ - provider: 'google_oauth2', - uid: '123', - info: { - name: 'dummy', - email: 'dummy@dummy.com' - }, - credentials: { - token: 'dummy', - expires_at: 123456 - } - }) -<<<<<<< HEAD - -======= + OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new({ + provider: 'google_oauth2', + uid: '123', + info: { + name: 'dummy', + email: 'dummy@dummy.com' + }, + credentials: { + token: 'dummy', + expires_at: 123456 + } + }) before_user_count = User.count - ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 user = User.from_omniauth(OmniAuth.config.mock_auth[:google_oauth2]) expect(user.provider).to eq('google_oauth2') - expect(user.uid).to eq('123') - expect(user.name).to eq('dummy') - expect(user.email).to eq('dummy@dummy.com') - expect(user.oauth_token).to eq('dummy') - #expect(user.token_expires_at).to eq(123_456) -<<<<<<< HEAD - end - end + expect(user.uid).to eq('123') + expect(user.name).to eq('dummy') + expect(user.email).to eq('dummy@dummy.com') + expect(user.oauth_token).to eq('dummy') + #expect(user.token_expires_at).to eq(123_456) + + + end + end it "should have many daily_logs" do - t = User.reflect_on_association(:daily_logs) - t.macro.should == :has_many + association = User.reflect_on_association(:daily_logs) + association.macro.should == :has_many end it "should have many and belongs to projects" do - t = User.reflect_on_association(:projects) - t.macro.should == :has_and_belongs_to_many + association = User.reflect_on_association(:projects) + association.macro.should == :has_and_belongs_to_many end + #context 'when newly created' do + #it 'should return an empty collection for projects' do + #User.joins(:project).where('id=?',user.id).count.should == 0 + #end + #it 'should return an empty collection for daily_logs' do + #User.joins(:daily_logs).where('id=?',user.id).count.should == 0 + #end + ##expect(User.count).to eq(before_user_count+1) + #end - context 'when newly created' do - it 'should return an empty collection for projects' do - User.joins(:project).where('id=?',user.id).count.should == 0 - it 'should return an empty collection for daily_logs' do - User.joins(:daily_logs).where('id=?',user.id).count.should == 0 + # pending 'should not create a new user if the user already exists' + context 'when authentication fails' do + # describe what happens when authentication fails. end -======= - - expect(User.count).to eq(before_user_count+1) - end - - pending 'should not create a new user if the user already exists' - context 'when authentication fails' do - # describe what happens when authentication fails. - end - - end - - it 'should have many projects' - - it 'should have many daily_logs' +end - context 'when newly created' do - it 'should return an empty collection for projects' - it 'should return an empty collection for daily_logs' - end - #pending "add some examples to (or delete) #{__FILE__}" ->>>>>>> 3b127cb9e81258aac7731555728298c01f41f6d8 -end +#pending "add some examples to (or delete) #{__FILE__}" From 9a53b7c8af9782d323f4d829bce1c9da7e8f9e3f Mon Sep 17 00:00:00 2001 From: saichander Date: Thu, 22 Jun 2017 16:22:46 +0530 Subject: [PATCH 04/17] daily_logger: code review comments --- app/controllers/application_controller.rb | 1 + app/controllers/daily_logs_controller.rb | 63 +++++++++++------------ app/helpers/daily_logs_helper.rb | 8 ++- app/models/daily_log.rb | 40 +++++++++----- app/views/daily_logs/new.html.haml | 2 + app/views/daily_logs/people_logs.js | 5 +- app/views/daily_logs/summary.html.haml | 1 + 7 files changed, 69 insertions(+), 51 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4338298..8c7f85e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class ApplicationController < ActionController::Base + #TODO add authencticate_user in before action protect_from_forgery with: :exception helper_method :current_user diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index 002bf2a..db9afc6 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -1,56 +1,46 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token + def new + #TODO authencticate user in application controller if(current_user.nil?) url="/auth/google_oauth2" redirect_to(url) else - #@projects=Project.where(client_name: Project::BEAUTIFUL_CODE_CLIENT) @daily_log =current_user.daily_logs.new @projects=Project.all - #if(!params[:start_date].nil?) - #session[:start_date] =params[:start_date] - #session[:end_date]=params[:end_date] - #end - #if(params[:project_id].nil? != true ) - #if(params[:project_id].empty?) - #session[:project_id]=nil - #else - #session[:project_id] =params[:project_id] - #end - #end + #TODO move my logs related code to index action query_hash = build_query_hash(params) - # query_hash.merge!(user_id: current_user.id) - #if(session[:start_date] != nil) - #query_hash.merge!(log_date: session[:start_date]..session[:end_date]) - #else - #query_hash.merge!(log_date: 1.week.ago..Date.today) - #end - #@user_log_summary=@daily_log.user_create_summary(query_hash,session[:project_id]) + #move this logic to model method, you are calling same method twice with diff params if(params[:project_id].nil? or params[:project_id].empty?) - @user_log_summary=DailyLog.new.user_create_summary(query_hash,nil) - else - @user_log_summary=DailyLog.new.user_create_summary(query_hash,params[:project_id]) - end - + @user_log_summary=DailyLog.new.user_create_summary(query_hash,nil) + else + @user_log_summary=DailyLog.new.user_create_summary(query_hash,params[:project_id]) + end end - end def create if(DailyLog.where(user_id: current_user.id,log_date:params[:datetime_ida]).count==0) - ActiveRecord::Base.transaction do - daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) - params[:Log_entry].each do |project_id,log_texts| - log_texts.each do |logtext| - daily_log.log_entries.create(project_id:project_id.to_i,log_text:logtext) + begin + ActiveRecord::Base.transaction do + daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) + #TODO params should be in lower case, params[:log_entries] + params[:Log_entry].each do |project_id,log_texts| + log_texts.each do |logtext| + daily_log.log_entries.create(project_id:project_id.to_i,log_text:logtext) + end end + #TODO params should be in lower case + ##TODO add this to above log entries + daily_log.log_entries.new.create(project_id: 4,log_text: params[:LearningLog]) end - daily_log.log_entries.new.create_logentry(4,params[:LearningLog]) + rescue + #TODO add flash error message and redirect to respective path end - flash[:success]="Daily log recorded" + flash[:success]="Daily log record is successfully created" redirect_to summary_path else #TODO handle case when records are not saved, show flash message @@ -70,14 +60,18 @@ def summary def people_logs @log_summary=DailyLog.new.create_summary(log_date: Date.today) + #TODO use lower case for instance variables @All_users=User.all; end + #TODO add empty line after every action def refresh @log_summary=DailyLog.new.create_summary(log_date: params[:log_date]) render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} end def user_logs + #TODO move build_query_hash helper method to model query_hash = build_query_hash(params) + #same changes as mentioned in new action if(params[:project_id].nil? or params[:project_id].empty?) @user_logs=DailyLog.new.user_create_summary(query_hash,nil) else @@ -89,17 +83,22 @@ def edit_log @edit_log = DailyLog.find(params[:log_id]).create_record(); @projects=Project.all @daily_log_id=params[:log_id] + #TODO remove unnecessary empty lines end + #TODO change this action name to update def finish_edit + #TODO change naming for params DailyLog.find_by_id(params[:daily_log_id]).update_records(params[:Logtext],params[:Learning_log],params[:takeaway]) if(!params[:NewLogtext].nil?) + #TODO use proper naming params[:NewLogtext].each do |project_id,log_entries| log_entries.each do|logtext| DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) end end end + #TODO handle negative case redirect_to new_daily_log_path end diff --git a/app/helpers/daily_logs_helper.rb b/app/helpers/daily_logs_helper.rb index 127ab10..07de72c 100644 --- a/app/helpers/daily_logs_helper.rb +++ b/app/helpers/daily_logs_helper.rb @@ -1,11 +1,9 @@ module DailyLogsHelper + #TODO move this code to model search method def build_query_hash(params) query_hash ={} - if(!params[:user_id].nil?) - query_hash.merge!(user_id: params[:user_id]) - else - query_hash.merge!(user_id: current_user.id) - end + query_hash[:user_id] = params[:user_id] || current_user.id + #TODO refactor below code if(!params[:start_date].nil?) query_hash.merge!(log_date: params[:start_date]..params[:end_date]) else diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index ee84adb..178780c 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -2,22 +2,26 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all belongs_to :user include ApplicationHelper - def user_create_summary(query_hash,project_id)# + + #TODO make this as a class method + def user_create_summary(query_hash,project_id) if(!project_id.nil?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("project_id=#{project_id}").order("log_date DESC").distinct + daily_logs= DailyLog.where(query_hash).joins(:log_entries).where(project_id: project_id).order("log_date DESC").distinct + #daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("project_id=#{project_id}").order("log_date DESC").distinct #daily_logs =DailyLog.joins(:log_entries).where(query_hash).where("log_entries.project_id=#{project_id}").order("log_date DESC").distinct else daily_logs=DailyLog.where(query_hash).order("log_date DESC") - end + + #TODO name it as daily log summary log_summary=Hash.new daily_logs.each_with_index do |daily_log,index| - summary_record=daily_log.create_record() - log_summary[index]=summary_record + log_summary[index] = daily_log.create_record() end log_summary end + #TODO change above method to take care of this functionality def create_summary(log_date:) daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") log_summary=Hash.new @@ -28,29 +32,37 @@ def create_summary(log_date:) log_summary end + #TODO dont use () when there are no args def create_record() summary_record=Hash.new - summary_record[:name] = User.where("id=#{self.user_id}")[0].name; + summary_record[:name] = self.user.name + #TODO remove semi colons and refactor below code , refer above line summary_record[:user_id]= User.where("id=#{self.user_id}")[0].id; summary_record[:picture]= User.where("id=#{self.user_id}")[0].picture; - summary_record[:log_id] = self.id; + #TODO use daily log record for edit log link , dont send log_id + summary_record[:log_id] = self.id summary_record[:takeaway]=self.takeaway.to_s - summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; - summary_record[:beautifulcode]=self.company_record() + #summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; + summary_record[:learning]= self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text + summary_record[:beautifulcode] = company_record summary_record[:clients] =self.client_record() + #TODO add space before and after =, eg: a = b summary_record[:logdate]=self.log_date.strftime('%v') summary_record end + def company_record() company_record={} #TODO move where clauses to models instance methods project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id,:project_id) - project_log_logtext = Hash.new{|h,k| h[k] = []} + project_log_logtext = Hash.new{ |h,k| h[k] = [] } + #TODO use proper naming project_log_ids.each do |x| - project_log_logtext[Project.find_by_id(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] + project_log_logtext[Project.find(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] end project_log_logtext end + #TODO add empty line after each method def client_record() clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') project_logs=Hash.new(); @@ -64,10 +76,12 @@ def client_record() end project_logs end + #TODO add empty line after each method + #TODO change methods name to suitable name(update_daily_log_and_log_entries) def update_records(records,learning,takeaway) records.each do |project_id,log_entries| - log_entries.each do|logentry_id,log_text| - self.log_entries.find_by_id(logentry_id).update_attributes(project_id: project_id,log_text:log_text); + log_entries.each do|log_entry_id,log_text| + self.log_entries.find_by_id(log_entry_id).update_attributes(project_id: project_id,log_text:log_text); end end self.update_attributes(:takeaway => takeaway) diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index 749ef83..d51d14f 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -1,6 +1,7 @@ %div.container-fluid %div.row %div.col-md-4 + -#remove commented out code -#%input{ "type"=>"text","name"=>"daterange"} -#%i.fa.fa-calendar.form-control-feedback #calendar2.ui @@ -11,6 +12,7 @@ %div.col-md-4 = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil,:class => "projects_filter"} -if(@user_log_summary[0][:logdate] !=Date.today().strftime("%v")) + -#move this code to partial %div.row %div.col-md-8.page = form_tag daily_logs_path, :method => :post do diff --git a/app/views/daily_logs/people_logs.js b/app/views/daily_logs/people_logs.js index cdc8c3f..635130e 100644 --- a/app/views/daily_logs/people_logs.js +++ b/app/views/daily_logs/people_logs.js @@ -1 +1,4 @@ -window.location.reload(); \ No newline at end of file +/* + *delete deprecated files + */ +window.location.reload(); diff --git a/app/views/daily_logs/summary.html.haml b/app/views/daily_logs/summary.html.haml index 6acf820..55bf941 100644 --- a/app/views/daily_logs/summary.html.haml +++ b/app/views/daily_logs/summary.html.haml @@ -1,3 +1,4 @@ +-#remove deprecated files %h1 Summary =render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} From fd8816b35e7bf7cfef76c3dcd657505ae244d315 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 27 Jun 2017 23:38:31 +0530 Subject: [PATCH 05/17] bug fixes and model specs wip --- app/assets/javascripts/daily_log.js | 7 +- app/assets/javascripts/edit_log.js | 8 +- app/controllers/application_controller.rb | 7 ++ app/controllers/daily_logs_controller.rb | 89 +++++++----------- app/helpers/daily_logs_helper.rb | 12 --- app/models/application_record.rb | 2 +- app/models/daily_log.rb | 94 +++++++++---------- .../daily_logs/_log_entry_fields.html.haml | 6 -- app/views/daily_logs/_user_list.html.haml | 2 +- app/views/daily_logs/edit_log.html.haml | 10 +- app/views/daily_logs/new.html.haml | 5 +- app/views/daily_logs/people_logs.js | 4 - config/routes.rb | 2 +- spec/models/dailylog_spec.rb | 8 +- 14 files changed, 104 insertions(+), 152 deletions(-) delete mode 100644 app/views/daily_logs/_log_entry_fields.html.haml delete mode 100644 app/views/daily_logs/people_logs.js diff --git a/app/assets/javascripts/daily_log.js b/app/assets/javascripts/daily_log.js index 946dfbe..00096b8 100644 --- a/app/assets/javascripts/daily_log.js +++ b/app/assets/javascripts/daily_log.js @@ -2,7 +2,7 @@ function DailyLog() { $(document).delegate('.add_field_button', 'click', function() { var project_id= $(this).parent().parent().children().eq(0).val(); - $(this).parent().children().eq(1).append("
"); + $(this).parent().children().eq(1).append("
"); }); $(document).delegate('.add_project_button', 'click', function() { @@ -14,9 +14,6 @@ function DailyLog() { $(document).ready(function() { $('input[name="daterange"]').daterangepicker(); - //$('#datetime_ida').datepicker({ - //endDate: "+0d"; - //}) }); $('input[name="daterange"]').on('apply.daterangepicker', function(ev, picker) { var project_id= $("#projects_filter").val(); @@ -69,7 +66,7 @@ function DailyLog() { $(document).delegate('.added', 'change', function() { var project_id =$(this).val(); - var name= 'Log_entry['+project_id+'][]'; + var name= 'log_entry['+project_id+'][]'; $(this).next().next().next().children().eq(1).children().filter('.form-control').attr('name',name); $(this).next().next().next().children().filter('.form-control').attr('name',name); }); diff --git a/app/assets/javascripts/edit_log.js b/app/assets/javascripts/edit_log.js index bc0b7d1..393d591 100644 --- a/app/assets/javascripts/edit_log.js +++ b/app/assets/javascripts/edit_log.js @@ -6,8 +6,7 @@ function EditLog() { { var project_id= $(this).parent().parent().children().eq(0).children().attr("value"); } - - $(this).parent().children().eq(0).append("
"); + $(this).parent().children().eq(0).append("
"); }); $(document).delegate('.added', 'change', function() { @@ -16,18 +15,17 @@ function EditLog() { { project_id= $(this).attr("value"); } - var name= 'NewLogtext['+project_id+'][]'; + var name= 'new_log_entries['+project_id+'][]'; $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.NewlyAdded').attr('name',name); $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.existing').each(function(i){ var oldName= $(this).attr('name'); var newName='Logtext['+project_id+']'+oldName.substring(oldName.lastIndexOf('[')); $(this).attr('name',newName); }); - //$(this).next().next().next().children().filter('.form-control').attr('name',name); }); $(document).delegate('.add_project_button','click',function() { - var value= $('#projects_').attr('value'); + var value= $('#projects_').attr('value'); var attrid ="projects[]" $('.project').append("
Task

"); $('.newproject').attr('name',attrid).append($('#projects_').html()); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8c7f85e..3634031 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,4 +9,11 @@ class ApplicationController < ActionController::Base def current_user @current_user ||= User.find(session[:user_id]) if session[:user_id] end + def authenticate_user + if (current_user.nil?) + url="/auth/google_oauth2" + redirect_to(url) + end + end + end diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index db9afc6..f530222 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -1,25 +1,13 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token + before_action :authenticate_user, :except=>[:index] def new - #TODO authencticate user in application controller - if(current_user.nil?) - url="/auth/google_oauth2" - redirect_to(url) - else - @daily_log =current_user.daily_logs.new - @projects=Project.all - - #TODO move my logs related code to index action - query_hash = build_query_hash(params) - #move this logic to model method, you are calling same method twice with diff params - if(params[:project_id].nil? or params[:project_id].empty?) - @user_log_summary=DailyLog.new.user_create_summary(query_hash,nil) - else - @user_log_summary=DailyLog.new.user_create_summary(query_hash,params[:project_id]) - end - end + @daily_log =current_user.daily_logs.new + @projects=Project.all + query_hash = DailyLog.build_query_hash(params,current_user.id) + @user_log_summary=DailyLog.user_create_summary(query_hash,params[:project_id],nil) end def create @@ -27,80 +15,69 @@ def create begin ActiveRecord::Base.transaction do daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) - #TODO params should be in lower case, params[:log_entries] - params[:Log_entry].each do |project_id,log_texts| + params[:log_entry].each do |project_id,log_texts| log_texts.each do |logtext| daily_log.log_entries.create(project_id:project_id.to_i,log_text:logtext) end end - #TODO params should be in lower case - ##TODO add this to above log entries - daily_log.log_entries.new.create(project_id: 4,log_text: params[:LearningLog]) end rescue - #TODO add flash error message and redirect to respective path + flash[:info]= "Error occured,please try again" + redirect_to new_daily_log_path end flash[:success]="Daily log record is successfully created" - redirect_to summary_path + redirect_to new_daily_log_path else - #TODO handle case when records are not saved, show flash message flash[:info]= "You have already entered daily_log for this day" redirect_to new_daily_log_path end - end def index end #TODO change this action to index action - def summary - @log_summary=DailyLog.new.create_summary(log_date: Date.today) - end def people_logs - @log_summary=DailyLog.new.create_summary(log_date: Date.today) - #TODO use lower case for instance variables - @All_users=User.all; + @log_summary=DailyLog.user_create_summary(nil,nil,Date.today) + @all_users=User.all; end - #TODO add empty line after every action + def refresh - @log_summary=DailyLog.new.create_summary(log_date: params[:log_date]) + @log_summary=DailyLog.user_create_summary(nil,nil, params[:log_date]) render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} end + def user_logs - #TODO move build_query_hash helper method to model - query_hash = build_query_hash(params) - #same changes as mentioned in new action - if(params[:project_id].nil? or params[:project_id].empty?) - @user_logs=DailyLog.new.user_create_summary(query_hash,nil) - else - @user_logs=DailyLog.new.user_create_summary(query_hash,params[:project_id]) - end + query_hash = DailyLog.build_query_hash(params,current_user.id) + @user_logs=DailyLog.user_create_summary(query_hash,params[:project_id],nil) render :partial => "daily_logs/user_summary",object: @user_logs,locals: { log_summary: @user_logs} end + def edit_log - @edit_log = DailyLog.find(params[:log_id]).create_record(); + @edit_log = DailyLog.find(params[:log_id]).create_record(nil) @projects=Project.all @daily_log_id=params[:log_id] - #TODO remove unnecessary empty lines - end - #TODO change this action name to update - def finish_edit - #TODO change naming for params - DailyLog.find_by_id(params[:daily_log_id]).update_records(params[:Logtext],params[:Learning_log],params[:takeaway]) - if(!params[:NewLogtext].nil?) - #TODO use proper naming - params[:NewLogtext].each do |project_id,log_entries| - log_entries.each do|logtext| - DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) + + def update + begin + ActiveRecord::Base.transaction do + DailyLog.find(params[:daily_log_id]).update_daily_log_and_log_entries(params[:log_entries],params[:learning_log],params[:takeaway]) + if(!params[:new_log_entries].nil?) + params[:new_log_entries].each do |project_id,log_entries| + log_entries.each do|logtext| + DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) + end + end end end + rescue + flash[:info] = "Update failed,Please try again" + redirect_to new_daily_log_path end - #TODO handle negative case + flash[:info] = "Updated successfully" redirect_to new_daily_log_path end - end diff --git a/app/helpers/daily_logs_helper.rb b/app/helpers/daily_logs_helper.rb index 07de72c..46962b1 100644 --- a/app/helpers/daily_logs_helper.rb +++ b/app/helpers/daily_logs_helper.rb @@ -1,16 +1,4 @@ module DailyLogsHelper #TODO move this code to model search method - def build_query_hash(params) - query_hash ={} - query_hash[:user_id] = params[:user_id] || current_user.id - #TODO refactor below code - if(!params[:start_date].nil?) - query_hash.merge!(log_date: params[:start_date]..params[:end_date]) - else - query_hash.merge!(log_date: 1.week.ago..Date.today) - end - query_hash - end -end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 71fbba5..2148120 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -2,4 +2,4 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true -end + end diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 178780c..695079a 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -3,71 +3,59 @@ class DailyLog < ApplicationRecord belongs_to :user include ApplicationHelper - #TODO make this as a class method - def user_create_summary(query_hash,project_id) - if(!project_id.nil?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where(project_id: project_id).order("log_date DESC").distinct - #daily_logs= DailyLog.where(query_hash).joins(:log_entries).where("project_id=#{project_id}").order("log_date DESC").distinct - #daily_logs =DailyLog.joins(:log_entries).where(query_hash).where("log_entries.project_id=#{project_id}").order("log_date DESC").distinct + def self.user_create_summary(query_hash,project_id,log_date) + if(!project_id.nil? and !project_id.blank?) + daily_logs= DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct else daily_logs=DailyLog.where(query_hash).order("log_date DESC") end - - #TODO name it as daily log summary - log_summary=Hash.new - daily_logs.each_with_index do |daily_log,index| - log_summary[index] = daily_log.create_record() + if(query_hash.nil? and project_id.nil? and !log_date.nil? ) + daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") end - log_summary - end - - #TODO change above method to take care of this functionality - def create_summary(log_date:) - daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") - log_summary=Hash.new + daily_log_summary=Hash.new daily_logs.each_with_index do |daily_log,index| - summary_record = daily_log.create_record() - log_summary[index]=summary_record + daily_log_summary[index] = daily_log.create_record(project_id) end - log_summary + daily_log_summary end - #TODO dont use () when there are no args - def create_record() + def create_record(project_id) summary_record=Hash.new summary_record[:name] = self.user.name - #TODO remove semi colons and refactor below code , refer above line - summary_record[:user_id]= User.where("id=#{self.user_id}")[0].id; - summary_record[:picture]= User.where("id=#{self.user_id}")[0].picture; - #TODO use daily log record for edit log link , dont send log_id + summary_record[:user_id] = self.user.id + summary_record[:picture] = self.user.picture summary_record[:log_id] = self.id - summary_record[:takeaway]=self.takeaway.to_s - #summary_record[:learning]= LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='Learning'")[0].log_text; - summary_record[:learning]= self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text - summary_record[:beautifulcode] = company_record - summary_record[:clients] =self.client_record() - #TODO add space before and after =, eg: a = b - summary_record[:logdate]=self.log_date.strftime('%v') + summary_record[:takeaway] = self.takeaway.to_s + summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text + summary_record[:beautifulcode] = beautifulcode_log_record(project_id) + summary_record[:clients] = self.clients_project_log_record(project_id) + summary_record[:logdate] = self.log_date.strftime('%v') summary_record end - def company_record() + def beautifulcode_log_record(project_id) company_record={} - #TODO move where clauses to models instance methods - project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id,:project_id) + if(!project_id.nil? and !project_id.blank?) + project_log_ids = self.log_entries.joins(:project).where(projects:{id: project_id}).pluck(:id,:project_id) + else + project_log_ids=self.log_entries.joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id,:project_id) + end project_log_logtext = Hash.new{ |h,k| h[k] = [] } - #TODO use proper naming - project_log_ids.each do |x| - project_log_logtext[Project.find(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] + project_log_ids.each do |project_log_ids| + project_log_logtext[Project.find(project_log_ids[1]).name] <<[LogEntry.find_by_id(project_log_ids[0]).log_text,project_log_ids[0]] end project_log_logtext end - #TODO add empty line after each method - def client_record() + + def clients_project_log_record(project_id) clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') project_logs=Hash.new(); clients.each do |client| - project_log_ids=LogEntry.where(:daily_log_id => self.id).joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + if(!project_id.nil? and !project_id.blank?) + project_log_ids= self.log_entries.joins(:project).where(projects:{id: project_id, client_name:'#{client}' }).pluck(:id,:project_id) + else + project_log_ids=self.log_entries.joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + end project_log_logtext = Hash.new{|h,k| h[k] = []} project_log_ids.each do |x| project_log_logtext[Project.find_by_id(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] @@ -76,15 +64,25 @@ def client_record() end project_logs end - #TODO add empty line after each method - #TODO change methods name to suitable name(update_daily_log_and_log_entries) - def update_records(records,learning,takeaway) - records.each do |project_id,log_entries| + + def update_daily_log_and_log_entries(log_entry_records,learning,takeaway) + log_entry_records.each do |project_id,log_entries| log_entries.each do|log_entry_id,log_text| - self.log_entries.find_by_id(log_entry_id).update_attributes(project_id: project_id,log_text:log_text); + self.log_entries.find(log_entry_id).update_attributes(project_id: project_id,log_text:log_text); end end self.update_attributes(:takeaway => takeaway) self.log_entries.find_by_project_id(4).update_attributes(log_text: learning) end + def self.build_query_hash(params,current_user_id) + query_hash ={} + query_hash[:user_id] = params[:user_id] || current_user_id + #TODO refactor below code + if(!params[:start_date].nil?) + query_hash.merge!(log_date: params[:start_date]..params[:end_date]) + else + query_hash.merge!(log_date: 1.week.ago..Date.today) + end + query_hash + end end diff --git a/app/views/daily_logs/_log_entry_fields.html.haml b/app/views/daily_logs/_log_entry_fields.html.haml deleted file mode 100644 index 2e9a42b..0000000 --- a/app/views/daily_logs/_log_entry_fields.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -.nested-fields - .field - = f.label :description - %br - = f.text_field :log_text - = link_to_remove_association "remove task", f \ No newline at end of file diff --git a/app/views/daily_logs/_user_list.html.haml b/app/views/daily_logs/_user_list.html.haml index 4246390..0f2728e 100644 --- a/app/views/daily_logs/_user_list.html.haml +++ b/app/views/daily_logs/_user_list.html.haml @@ -1,5 +1,5 @@ .container - - @All_users.each_slice(4).each do |users| + - @all_users.each_slice(4).each do |users| .row - users.each do |user| .col-sm-3.col-xs-6.hover-cover diff --git a/app/views/daily_logs/edit_log.html.haml b/app/views/daily_logs/edit_log.html.haml index 2bb5f98..ba253e7 100644 --- a/app/views/daily_logs/edit_log.html.haml +++ b/app/views/daily_logs/edit_log.html.haml @@ -1,7 +1,7 @@ %div.user_summary %div.row %div.col-md-8.page - =form_tag finish_edit_path, :method => :post do + =form_tag update_path, :method => :post do .row .col-md-1 = image_tag (image_url(@edit_log[:picture] )), class:"imagecircle" @@ -21,9 +21,9 @@ %div %div -logtexts.each do |logtext| - =text_field_tag("Logtext["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") + =text_field_tag("log_entries["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") %br - %button.btn.btn-default.add_field_button{"id"=>"company"} + %button.btn.btn-default.add_field_button Add Task %br %div.project @@ -37,7 +37,7 @@ %div %div -logtexts.each do |logtext| - =text_field_tag("Logtext["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") + =text_field_tag("log_entries["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") %br %button.btn.btn-default.add_field_button{"id"=>"company"} Add Task @@ -49,7 +49,7 @@ %h5 Learning %p - =text_field_tag("Learning_log","#{@edit_log[:learning]}",:class => "form-control") + =text_field_tag("learning_log","#{@edit_log[:learning]}",:class => "form-control") %h5 TakeAway %p diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index d51d14f..68e8107 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -1,9 +1,6 @@ %div.container-fluid %div.row %div.col-md-4 - -#remove commented out code - -#%input{ "type"=>"text","name"=>"daterange"} - -#%i.fa.fa-calendar.form-control-feedback #calendar2.ui .ui.input.left.icon %i.calendar.icon @@ -47,7 +44,7 @@ Learning %p what did you learn today? - %input.form-control{"name"=>"learning_log","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} + %input.form-control{"name"=>"log_entry[4][]","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} %hr %div %h3 diff --git a/app/views/daily_logs/people_logs.js b/app/views/daily_logs/people_logs.js deleted file mode 100644 index 635130e..0000000 --- a/app/views/daily_logs/people_logs.js +++ /dev/null @@ -1,4 +0,0 @@ -/* - *delete deprecated files - */ -window.location.reload(); diff --git a/config/routes.rb b/config/routes.rb index f876c36..035566d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,7 +11,7 @@ get 'daily_logs/user_list', :to => 'daily_logs#user_list' match 'daily_logs/user_logs', :to => 'daily_logs#user_logs', via:[:GET,:POST] match 'daily_logs/edit_log', :to => 'daily_logs#edit_log', via:[:GET,:POST],as:'edit_log' - match 'daily_logs/finish_edit', :to =>'daily_logs#finish_edit',via: [:POST], as:'finish_edit' + match 'daily_logs/update', :to =>'daily_logs#update',via: [:POST], as:'update' resources :sessions, only: [:create, :destroy] resources :daily_logs, only: [:new, :create] diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index 2a3ad61..754f4ef 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -9,12 +9,12 @@ end it "should have many log_entries" do - t = DailyLog.reflect_on_association(:log_entries) - t.macro.should == :has_many + association = DailyLog.reflect_on_association(:log_entries) + association.macro.should == :has_many end it 'should belong to a user' do - t = DailyLog.reflect_on_association(:user) - t.macro.should == :belongs_to + association = DailyLog.reflect_on_association(:user) + association.macro.should == :belongs_to end # describe :create_record do # log=DailyLog.new From 3fb6882dadd0b52fe3a3009aeb208b96286d9b12 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Tue, 27 Jun 2017 23:51:52 +0530 Subject: [PATCH 06/17] bugfix form show for new users --- app/views/daily_logs/new.html.haml | 91 +++++++++++++++--------------- config/routes.rb | 3 - spec/models/user_spec.rb | 6 +- 3 files changed, 48 insertions(+), 52 deletions(-) diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index 68e8107..1c350bf 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -8,57 +8,58 @@ %div.row %div.col-md-4 = select_tag "projects_filter", options_for_select(@projects.map {|u| [u.name,u.id]}),{:include_blank => "All", :default => nil,:class => "projects_filter"} --if(@user_log_summary[0][:logdate] !=Date.today().strftime("%v")) +-#if(!@user_log_summary.empty? && (@user_log_summary[0][:logdate] !=Date.today().strftime("%v")) -#move this code to partial - %div.row - %div.col-md-8.page - = form_tag daily_logs_path, :method => :post do - .row - .col-md-1 - = image_tag (image_url(current_user.picture )), class:"imagecircle" - .col-md-4 - %h4.centeralign - =current_user.name - .col-md-7.pull-right - =date_field_tag(:datetime_ida, Date.today) - %h3.headerclass - Work - %div.project - = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:class => "added",:prompt => "Select Project", :required => "required" - %br - %h5.task - Task +%div.row + %div.col-md-8.page + = form_tag daily_logs_path, :method => :post do + .row + .col-md-1 + = image_tag (image_url(current_user.picture )), class:"imagecircle" + .col-md-4 + %h4.centeralign + =current_user.name + .col-md-7.pull-right + =date_field_tag(:datetime_ida, Date.today) + %h3.headerclass + Work + %div.project + = select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:class => "added",:prompt => "Select Project", :required => "required" + %br + %h5.task + Task + %div + %input.form-control{"maxlength"=>"300","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} %div - %input.form-control{"maxlength"=>"300","required"=>"required","type"=>"text","placeholder" =>" enter LogEntry"} - %div - %br - %button.btn.btn-default.add_field_button{"id"=>"company"} - Add Task + %br + %button.btn.btn-default.add_field_button{"id"=>"company"} + Add Task + %hr + %div + %button.btn.btn-default.add_project_button{} + Add Project + %hr + %div + %h3 + Learning + %p + what did you learn today? + %input.form-control{"name"=>"log_entry[4][]","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} %hr %div - %button.btn.btn-default.add_project_button{} - Add Project + %h3 + Takeaways + %p + what are your takeaways? + %input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} %hr - %div - %h3 - Learning - %p - what did you learn today? - %input.form-control{"name"=>"log_entry[4][]","required"=>"required","type"=>"text","placeholder" =>" enter learning log "} - %hr - %div - %h3 - Takeaways - %p - what are your takeaways? - %input.form-control{"name"=>"takeaway","required"=>"required","type"=>"text","placeholder" =>" takeaways for the day "} - %hr - = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) - %br - %br - %br + = submit_tag("Submit", class: "btn btn-change btn-md btn-md pull-right" ) +%br +%br +%br .user_summary - =render :partial => "daily_logs/user_summary",object: @user_log_summary,locals: { log_summary: @user_log_summary} + - if @user_log_summary + =render :partial => "daily_logs/user_summary",object: @user_log_summary,locals: { log_summary: @user_log_summary} diff --git a/config/routes.rb b/config/routes.rb index 035566d..687a995 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,9 +15,6 @@ resources :sessions, only: [:create, :destroy] resources :daily_logs, only: [:new, :create] - # resources :daily_logs do - # get "/daily_logs/new", on: :collection - # end resources :projects root :to => 'daily_logs#index' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2a01315..e94f42f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -48,9 +48,7 @@ #end - # pending 'should not create a new user if the user already exists' - - context 'when authentication fails' do + context 'when authentication fails' do # describe what happens when authentication fails. end @@ -58,5 +56,5 @@ -#pending "add some examples to (or delete) #{__FILE__}" + From 41d1adc6c60b46107611bb704d0b5aafd7fd3832 Mon Sep 17 00:00:00 2001 From: saichander Date: Wed, 28 Jun 2017 21:36:07 +0530 Subject: [PATCH 07/17] code review comments 28th June --- app/controllers/daily_logs_controller.rb | 14 +++++++++++--- app/models/daily_log.rb | 20 +++++++++++++++++++- app/views/daily_logs/index.html.haml | 1 + app/views/daily_logs/new.html.haml | 1 + app/views/daily_logs/summary.html.haml | 2 +- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index f530222..7982106 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -6,7 +6,9 @@ class DailyLogsController < ApplicationController def new @daily_log =current_user.daily_logs.new @projects=Project.all + #TODO call build_query_hash in user_create_summary method itself query_hash = DailyLog.build_query_hash(params,current_user.id) + #TODO empty space before and after = @user_log_summary=DailyLog.user_create_summary(query_hash,params[:project_id],nil) end @@ -14,7 +16,9 @@ def create if(DailyLog.where(user_id: current_user.id,log_date:params[:datetime_ida]).count==0) begin ActiveRecord::Base.transaction do - daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) + #TODO which one do you think is better? + daily_log = current_user.daily_logs.create(takeaway: params[:takeaway],log_date: params[:datetime_ida]) + #daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) params[:log_entry].each do |project_id,log_texts| log_texts.each do |logtext| daily_log.log_entries.create(project_id:project_id.to_i,log_text:logtext) @@ -22,6 +26,7 @@ def create end end rescue + #TODO it should be flash[:error] flash[:info]= "Error occured,please try again" redirect_to new_daily_log_path end @@ -36,8 +41,6 @@ def create def index end - #TODO change this action to index action - def people_logs @log_summary=DailyLog.user_create_summary(nil,nil,Date.today) @all_users=User.all; @@ -45,16 +48,20 @@ def people_logs def refresh @log_summary=DailyLog.user_create_summary(nil,nil, params[:log_date]) + #TODO why are you sending log_summary twice render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} end def user_logs + #TODO call build_query_hash in user_create_summary method itself query_hash = DailyLog.build_query_hash(params,current_user.id) @user_logs=DailyLog.user_create_summary(query_hash,params[:project_id],nil) + #TODO why are you sending user_logs twice render :partial => "daily_logs/user_summary",object: @user_logs,locals: { log_summary: @user_logs} end def edit_log + #TODO params should be params[:daily_log_id] @edit_log = DailyLog.find(params[:log_id]).create_record(nil) @projects=Project.all @daily_log_id=params[:log_id] @@ -67,6 +74,7 @@ def update if(!params[:new_log_entries].nil?) params[:new_log_entries].each do |project_id,log_entries| log_entries.each do|logtext| + #TODO no find_by_id, use find DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) end end diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 695079a..975b501 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -3,12 +3,16 @@ class DailyLog < ApplicationRecord belongs_to :user include ApplicationHelper + #TODO create_user_summary would have been a better name + #TODO golden rule: always space after a comma def self.user_create_summary(query_hash,project_id,log_date) + #TODO project_id.present? is same as !project_id.nil? if(!project_id.nil? and !project_id.blank?) daily_logs= DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct else daily_logs=DailyLog.where(query_hash).order("log_date DESC") end + #TODO query_hash.nil? will return false even if query_hash = {}, is your logic ok with this? if(query_hash.nil? and project_id.nil? and !log_date.nil? ) daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") end @@ -19,6 +23,7 @@ def self.user_create_summary(query_hash,project_id,log_date) daily_log_summary end + #TODO create_summary_record would be better name def create_record(project_id) summary_record=Hash.new summary_record[:name] = self.user.name @@ -26,6 +31,7 @@ def create_record(project_id) summary_record[:picture] = self.user.picture summary_record[:log_id] = self.id summary_record[:takeaway] = self.takeaway.to_s + #TODO dont hard code client name values summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text summary_record[:beautifulcode] = beautifulcode_log_record(project_id) summary_record[:clients] = self.clients_project_log_record(project_id) @@ -34,7 +40,9 @@ def create_record(project_id) end def beautifulcode_log_record(project_id) + #TODO change company_record name company_record={} + #TODO why two conditions, project_id.present? should be fine right? if(!project_id.nil? and !project_id.blank?) project_log_ids = self.log_entries.joins(:project).where(projects:{id: project_id}).pluck(:id,:project_id) else @@ -42,15 +50,22 @@ def beautifulcode_log_record(project_id) end project_log_logtext = Hash.new{ |h,k| h[k] = [] } project_log_ids.each do |project_log_ids| + #TODO why are you using find_by_id, when you have find() project_log_logtext[Project.find(project_log_ids[1]).name] <<[LogEntry.find_by_id(project_log_ids[0]).log_text,project_log_ids[0]] end project_log_logtext end def clients_project_log_record(project_id) - clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') + #TODO dont hard code client names + clients = Project.where.not(client_name: ['BeautifulCode', 'Learning']).distinct.pluck('client_name') + #clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') + #TODO no semi colons please project_logs=Hash.new(); clients.each do |client| + #TODO why two conditions, project_id.present? should be fine right? + ##TODO DONT repeat your code, DRY principle + ##TODO combine beautifulcode_log_record and clients_project_log_record, we dont have to show any difference for them if(!project_id.nil? and !project_id.blank?) project_log_ids= self.log_entries.joins(:project).where(projects:{id: project_id, client_name:'#{client}' }).pluck(:id,:project_id) else @@ -72,8 +87,11 @@ def update_daily_log_and_log_entries(log_entry_records,learning,takeaway) end end self.update_attributes(:takeaway => takeaway) + #TODO why are you hard coding project_id value 4 + ##TODO write a method in project model which return learning projects id self.log_entries.find_by_project_id(4).update_attributes(log_text: learning) end + #TODO add empty line after each method def self.build_query_hash(params,current_user_id) query_hash ={} query_hash[:user_id] = params[:user_id] || current_user_id diff --git a/app/views/daily_logs/index.html.haml b/app/views/daily_logs/index.html.haml index f364cf7..fc03f32 100644 --- a/app/views/daily_logs/index.html.haml +++ b/app/views/daily_logs/index.html.haml @@ -1,3 +1,4 @@ +-##TODO you are going to change this UX %pre - if current_user = link_to 'fill the daily log', new_daily_log_path, class: 'btn btn-primary btn-lg' diff --git a/app/views/daily_logs/new.html.haml b/app/views/daily_logs/new.html.haml index 1c350bf..024d602 100644 --- a/app/views/daily_logs/new.html.haml +++ b/app/views/daily_logs/new.html.haml @@ -59,6 +59,7 @@ %br .user_summary - if @user_log_summary + -##TODO why are sending @user_log_summary twice =render :partial => "daily_logs/user_summary",object: @user_log_summary,locals: { log_summary: @user_log_summary} diff --git a/app/views/daily_logs/summary.html.haml b/app/views/daily_logs/summary.html.haml index 55bf941..caf2f16 100644 --- a/app/views/daily_logs/summary.html.haml +++ b/app/views/daily_logs/summary.html.haml @@ -1,4 +1,4 @@ --#remove deprecated files +-#TODO remove deprecated files %h1 Summary =render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} From add0879b490bd8ca299987c2ac03e149882be602 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Thu, 29 Jun 2017 11:19:41 +0530 Subject: [PATCH 08/17] model specs --- app/models/daily_log.rb | 2 + app/models/user.rb | 2 + spec/factories.rb | 23 ++++++----- spec/models/daily_log_spec.rb | 32 ---------------- spec/models/dailylog_spec.rb | 72 +++++++++++++++++++++++++---------- spec/models/logentry_spec.rb | 2 + spec/models/user_spec.rb | 15 +++++++- 7 files changed, 82 insertions(+), 66 deletions(-) delete mode 100644 spec/models/daily_log_spec.rb diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 975b501..6ec4d0c 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -1,6 +1,7 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all belongs_to :user + validates :user_id, presence: true include ApplicationHelper #TODO create_user_summary would have been a better name @@ -36,6 +37,7 @@ def create_record(project_id) summary_record[:beautifulcode] = beautifulcode_log_record(project_id) summary_record[:clients] = self.clients_project_log_record(project_id) summary_record[:logdate] = self.log_date.strftime('%v') + byebug summary_record end diff --git a/app/models/user.rb b/app/models/user.rb index f9a5714..309158f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,8 @@ class User < ActiveRecord::Base has_and_belongs_to_many :projects has_many :daily_logs + validates :name, presence: true + validates :email, presence: true def self.from_omniauth(auth) where(provider: auth.provider, uid: auth.uid).first_or_initialize.tap do |user| diff --git a/spec/factories.rb b/spec/factories.rb index 3ad0175..538b0d4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -1,27 +1,26 @@ FactoryGirl.define do factory :user do name "John" - uid "3" + uid "1" email "abhipatnala@gmail.com" provider "google" end factory :daily_log do - log_date = '2017-05-31' - takeaway ="spend more time on learning" - user_id ="1" + log_date '2017-05-31' + takeaway 'spend more time on learning' + user_id '1' end factory :log_entry do - log_text ="done with views" - project_id ="2" - daily_log_id ="1" + log_text "done with views" + project_id "2" end factory :project do - name ="revdirect" - client_name ="Sojern" + name "revdirect" + client_name "Sojern" end factory :feedback do - feedback_text="good job" - user_id="2" - daily_log_id="2" + feedback_text "good job" + user_id "2" + daily_log_id "2" end end diff --git a/spec/models/daily_log_spec.rb b/spec/models/daily_log_spec.rb deleted file mode 100644 index a61164a..0000000 --- a/spec/models/daily_log_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'rails_helper' -RSpec.describe DailyLog, type: :model do - let(:daily_log) { FactoryGirl.build(:daily_log) } - [:log_date, :takeaway].each do |msg| - it "should respond to #{msg}" do - expect(daily_log).to respond_to(msg) - end - end - - it 'should belong to a user' - - it 'should have many log_entries' - - - - # describe :record_create do - # before do - # daily_log.user_id = user.id - # daily_log.save - # allow(daily_log).to receive(:company_record).and_return nil - # allow(daily_log).to receive(:client_record).and_return nil - # end - # it 'should return formatted daily log record' do - # record = daily_log.record_create - # #expect(record.keys).to match_array([:name, :takeaway, :learning, :beautifulcode, :clients]) - # #expect(record.keys).to match_array([:name]) - # end - # end -end - - - diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index 754f4ef..90168f1 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -1,8 +1,17 @@ require 'rails_helper' RSpec.describe DailyLog, type: :model do #pending "add some examples to (or delete) #{__FILE__}" + let(:user) {User.create(name:"avinash",email:"avinash@beautifulcode.in") } let(:daily_log) { FactoryGirl.build(:daily_log) } - [:log_date, :takeaway].each do |msg| + let(:log_entry) { FactoryGirl.build(:log_entry) } + before do + daily_log.user_id=user.id + daily_log.save + log_entry.daily_log_id=daily_log.id + log_entry.save + end + + [:log_date, :takeaway].each do |msg| it "should respond to #{msg}" do expect(daily_log).to respond_to(msg) end @@ -13,27 +22,50 @@ association.macro.should == :has_many end it 'should belong to a user' do - association = DailyLog.reflect_on_association(:user) + association = DailyLog.reflect_on_association(:user) association.macro.should == :belongs_to end - # describe :create_record do - # log=DailyLog.new - # before do - # log.id=1111 - # log.user_id = 1 - # log.log_date=Date.today - # log.takeaway="need to improve" - # log.save - # allow(log.class).to receive(:company_record).and_return nil - # allow(log.class).to receive(:client_record).and_return nil - # end - # it 'should return formatted daily log record' do - # #:log_date = Date.today - # record = log.create_summary - # expect(record.keys).to match_array([:name, :takeaway, :learning, :beautifulcode, :clients]) - # expect(record.keys).to match_array([:name]) - # end - # end + + it 'should have only one dailylog entry for given date for given user' do + @dailylog = DailyLog.where(log_date: Date.today,user_id: 1) + expect(@dailylog.count). to be <= 1 + end + it 'associated log entries should be destroyed on deletion' do + #count = DailyLog.first.log_entries.count + #total_count = LogEntry.all.count + #DailyLog.first.destroy + #expect(LogEntry.all.count).to equal(total_count - count) + #count =daily_log.log_entries.count + count=LogEntry.all.count + expect(count).to eq(1) + end + describe 'validations' do + it 'should have valid user_id' do + daily_log.update_attributes(user_id: nil) + expect(daily_log.errors).to include(:user_id) + end + + end + + describe :create_record do + log=DailyLog.new + query_hash={user_id: 1,log_date:1.week.ago..Date.today} + before do + log.id=1 + log.user_id = 1 + log.log_date=Date.today + log.takeaway="need to improve" + log.save + allow(log).to receive(:beautifulcode_log_record).and_return nil + allow(log).to receive(:clients_project_log_record).and_return nil + end + it 'should return formatted daily log record' do + #:log_date = Date.today + record = log.class.user_create_summary(query_hash,nil,nil) + expect(record.keys).to match_array([:name, :takeaway, :learning, :beautifulcode, :clients]) + expect(record.keys).to match_array([:name]) + end + end end diff --git a/spec/models/logentry_spec.rb b/spec/models/logentry_spec.rb index 323140f..3c2dc17 100644 --- a/spec/models/logentry_spec.rb +++ b/spec/models/logentry_spec.rb @@ -16,5 +16,7 @@ association = LogEntry.reflect_on_association(:project) association.macro.should == :belongs_to end + it "should have valid log_text" do + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e94f42f..489264c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe User, type: :model do - + let(:user) { User.create(name: 'test', email: 'test@beautifulcode.in') } describe :from_omniauth do it 'should get user details from oauth after successful authentication' do OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new({ @@ -37,6 +37,17 @@ association = User.reflect_on_association(:projects) association.macro.should == :has_and_belongs_to_many end + describe 'validations' do + it 'should have valid name' do + user.update_attributes(name: nil) + expect(user.errors).to include(:name) + end + + it 'should have valid email' do + user.update_attributes(email: nil) + expect(user.errors).to include(:email) + end + end #context 'when newly created' do #it 'should return an empty collection for projects' do #User.joins(:project).where('id=?',user.id).count.should == 0 @@ -48,7 +59,7 @@ #end - context 'when authentication fails' do + context 'when authentication fails' do # describe what happens when authentication fails. end From c9d331933871ba0f57b94dd0233724afcbe65f70 Mon Sep 17 00:00:00 2001 From: saichander Date: Thu, 29 Jun 2017 12:47:41 +0530 Subject: [PATCH 09/17] added specs for daily log model --- spec/models/dailylog_spec.rb | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index 90168f1..45cbea3 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -20,17 +20,24 @@ it "should have many log_entries" do association = DailyLog.reflect_on_association(:log_entries) association.macro.should == :has_many + + expect(association.macro).to eq(:has_many) end + it 'should belong to a user' do association = DailyLog.reflect_on_association(:user) + #TODO refactor this association.macro.should == :belongs_to end it 'should have only one dailylog entry for given date for given user' do @dailylog = DailyLog.where(log_date: Date.today,user_id: 1) - expect(@dailylog.count). to be <= 1 + #expect(@dailylog.count). to be <= 1 + + expect(@dailylog.count).to eq(1) end - it 'associated log entries should be destroyed on deletion' do + + it 'should delete all associated log entries on deleting daily log' do #count = DailyLog.first.log_entries.count #total_count = LogEntry.all.count #DailyLog.first.destroy @@ -47,6 +54,23 @@ end + describe :user_create_summary do + context "when project id is nil" do + it "should return daily logs for als projects" do + + end + end + + context "when project id is not nil" do + + end + + context "when project_id and query_hash are not present and log date is present" do + + end + + end + describe :create_record do log=DailyLog.new query_hash={user_id: 1,log_date:1.week.ago..Date.today} From 0ac0f6e539d32110c533ab3f62398d9f1571860b Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 1 Jul 2017 11:54:48 +0530 Subject: [PATCH 10/17] model test cases --- app/controllers/daily_logs_controller.rb | 48 +++----- app/models/daily_log.rb | 86 +++++--------- app/views/daily_logs/_user_summary.html.haml | 17 +-- app/views/daily_logs/edit_log.html.haml | 25 ++-- config/projects.yml | 13 +++ db/seeds.rb | 13 +-- spec/factories.rb | 7 +- spec/models/dailylog_spec.rb | 117 ++++++++++++++----- spec/models/project_spec.rb | 15 +-- 9 files changed, 183 insertions(+), 158 deletions(-) create mode 100644 config/projects.yml diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index 7982106..c7d5d94 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -4,21 +4,16 @@ class DailyLogsController < ApplicationController before_action :authenticate_user, :except=>[:index] def new - @daily_log =current_user.daily_logs.new - @projects=Project.all - #TODO call build_query_hash in user_create_summary method itself - query_hash = DailyLog.build_query_hash(params,current_user.id) - #TODO empty space before and after = - @user_log_summary=DailyLog.user_create_summary(query_hash,params[:project_id],nil) + @daily_log = current_user.daily_logs.new + @projects = Project.where("name != 'Learning'") + @user_log_summary = DailyLog.create_user_summary(params,current_user.id,params[:project_id],nil) end def create if(DailyLog.where(user_id: current_user.id,log_date:params[:datetime_ida]).count==0) begin ActiveRecord::Base.transaction do - #TODO which one do you think is better? daily_log = current_user.daily_logs.create(takeaway: params[:takeaway],log_date: params[:datetime_ida]) - #daily_log=DailyLog.create(user_id: current_user.id,takeaway: params[:takeaway],log_date: params[:datetime_ida]) params[:log_entry].each do |project_id,log_texts| log_texts.each do |logtext| daily_log.log_entries.create(project_id:project_id.to_i,log_text:logtext) @@ -26,14 +21,13 @@ def create end end rescue - #TODO it should be flash[:error] - flash[:info]= "Error occured,please try again" + flash[:info] = "Error occured,please try again" redirect_to new_daily_log_path end - flash[:success]="Daily log record is successfully created" + flash[:success] = "Daily log record is successfully created" redirect_to new_daily_log_path else - flash[:info]= "You have already entered daily_log for this day" + flash[:info] = "You have already entered daily_log for this day" redirect_to new_daily_log_path end end @@ -42,29 +36,24 @@ def index end def people_logs - @log_summary=DailyLog.user_create_summary(nil,nil,Date.today) - @all_users=User.all; + @log_summary = DailyLog.create_user_summary(nil,nil,nil,Date.today) + @all_users = User.all; end def refresh - @log_summary=DailyLog.user_create_summary(nil,nil, params[:log_date]) - #TODO why are you sending log_summary twice - render :partial => "daily_logs/user_summary",object: @log_summary,locals: { log_summary: @log_summary} + @log_summary = DailyLog.create_user_summary(nil,nil,nil, params[:log_date]) + render :partial => "daily_logs/user_summary",locals: { log_summary: @log_summary} end def user_logs - #TODO call build_query_hash in user_create_summary method itself - query_hash = DailyLog.build_query_hash(params,current_user.id) - @user_logs=DailyLog.user_create_summary(query_hash,params[:project_id],nil) - #TODO why are you sending user_logs twice - render :partial => "daily_logs/user_summary",object: @user_logs,locals: { log_summary: @user_logs} + @user_logs = DailyLog.create_user_summary(params,current_user.id,params[:project_id],nil) + render :partial => "daily_logs/user_summary",locals: { log_summary: @user_logs} end def edit_log - #TODO params should be params[:daily_log_id] - @edit_log = DailyLog.find(params[:log_id]).create_record(nil) - @projects=Project.all - @daily_log_id=params[:log_id] + @edit_log = DailyLog.find(params[:daily_log_id]).create_summary_record(nil) + @projects = Project.where("name != 'Learning'") + @daily_log_id = params[:daily_log_id] end def update @@ -74,18 +63,17 @@ def update if(!params[:new_log_entries].nil?) params[:new_log_entries].each do |project_id,log_entries| log_entries.each do|logtext| - #TODO no find_by_id, use find - DailyLog.find_by_id(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) + DailyLog.find(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) end end end end + flash[:info] = "Updated successfully" + redirect_to new_daily_log_path rescue flash[:info] = "Update failed,Please try again" redirect_to new_daily_log_path end - flash[:info] = "Updated successfully" - redirect_to new_daily_log_path end end diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 6ec4d0c..7bb61c4 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -1,31 +1,29 @@ class DailyLog < ApplicationRecord - has_many :log_entries, :class_name => 'LogEntry',:dependent => :delete_all + has_many :log_entries, :class_name => 'LogEntry', :dependent => :delete_all belongs_to :user validates :user_id, presence: true include ApplicationHelper - #TODO create_user_summary would have been a better name - #TODO golden rule: always space after a comma - def self.user_create_summary(query_hash,project_id,log_date) - #TODO project_id.present? is same as !project_id.nil? - if(!project_id.nil? and !project_id.blank?) - daily_logs= DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct + def self.create_user_summary(params,user_id, project_id, log_date) + if(params.present?) + query_hash = DailyLog.build_query_hash(params,user_id) + end + if( project_id.present? ) + daily_logs = DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct else - daily_logs=DailyLog.where(query_hash).order("log_date DESC") + daily_logs = DailyLog.where(query_hash).order("log_date DESC") end - #TODO query_hash.nil? will return false even if query_hash = {}, is your logic ok with this? if(query_hash.nil? and project_id.nil? and !log_date.nil? ) - daily_logs= DailyLog.where(log_date: log_date).order("log_date DESC") + daily_logs = DailyLog.where(log_date: log_date).order("log_date DESC") end - daily_log_summary=Hash.new + daily_log_summary = Hash.new daily_logs.each_with_index do |daily_log,index| - daily_log_summary[index] = daily_log.create_record(project_id) + daily_log_summary[index] = daily_log.create_summary_record(project_id) end daily_log_summary end - #TODO create_summary_record would be better name - def create_record(project_id) + def create_summary_record(project_id) summary_record=Hash.new summary_record[:name] = self.user.name summary_record[:user_id] = self.user.id @@ -33,49 +31,26 @@ def create_record(project_id) summary_record[:log_id] = self.id summary_record[:takeaway] = self.takeaway.to_s #TODO dont hard code client name values - summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text - summary_record[:beautifulcode] = beautifulcode_log_record(project_id) + summary_record[:learning]= self.log_entries.where(project_id:Project.where(client_name:'Learning')).first.log_text + # summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text summary_record[:clients] = self.clients_project_log_record(project_id) summary_record[:logdate] = self.log_date.strftime('%v') - byebug summary_record end - def beautifulcode_log_record(project_id) - #TODO change company_record name - company_record={} - #TODO why two conditions, project_id.present? should be fine right? - if(!project_id.nil? and !project_id.blank?) - project_log_ids = self.log_entries.joins(:project).where(projects:{id: project_id}).pluck(:id,:project_id) - else - project_log_ids=self.log_entries.joins(:project).where("projects.client_name='BeautifulCode'").pluck(:id,:project_id) - end - project_log_logtext = Hash.new{ |h,k| h[k] = [] } - project_log_ids.each do |project_log_ids| - #TODO why are you using find_by_id, when you have find() - project_log_logtext[Project.find(project_log_ids[1]).name] <<[LogEntry.find_by_id(project_log_ids[0]).log_text,project_log_ids[0]] - end - project_log_logtext - end - def clients_project_log_record(project_id) #TODO dont hard code client names - clients = Project.where.not(client_name: ['BeautifulCode', 'Learning']).distinct.pluck('client_name') - #clients=Project.where.not(client_name: ['BeautifulCode', 'Learning']).pluck('distinct client_name') - #TODO no semi colons please - project_logs=Hash.new(); + clients = Project.where.not(client_name: 'Learning').distinct.pluck('client_name') + project_logs = Hash.new() clients.each do |client| - #TODO why two conditions, project_id.present? should be fine right? - ##TODO DONT repeat your code, DRY principle - ##TODO combine beautifulcode_log_record and clients_project_log_record, we dont have to show any difference for them - if(!project_id.nil? and !project_id.blank?) - project_log_ids= self.log_entries.joins(:project).where(projects:{id: project_id, client_name:'#{client}' }).pluck(:id,:project_id) + if( project_id.present? ) + project_log_ids = self.log_entries.joins(:project).where(projects:{id: project_id, client_name:"#{client}" }).pluck(:id,:project_id) else - project_log_ids=self.log_entries.joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) + project_log_ids = self.log_entries.joins(:project).where("projects.client_name='#{client}'").pluck(:id,:project_id) end project_log_logtext = Hash.new{|h,k| h[k] = []} - project_log_ids.each do |x| - project_log_logtext[Project.find_by_id(x[1]).name] <<[LogEntry.find_by_id(x[0]).log_text,x[0]] + project_log_ids.each do |project_id_log_id| + project_log_logtext[Project.find_by_id(project_id_log_id[1]).name] <<[LogEntry.find_by_id(project_id_log_id[0]).log_text,project_id_log_id[0]] end project_logs[client.to_s]=project_log_logtext; end @@ -84,25 +59,24 @@ def clients_project_log_record(project_id) def update_daily_log_and_log_entries(log_entry_records,learning,takeaway) log_entry_records.each do |project_id,log_entries| - log_entries.each do|log_entry_id,log_text| - self.log_entries.find(log_entry_id).update_attributes(project_id: project_id,log_text:log_text); + log_entries.each do |log_entry_id,log_text| + self.log_entries.find(log_entry_id).update_attributes(project_id: project_id, log_text:log_text); end end self.update_attributes(:takeaway => takeaway) - #TODO why are you hard coding project_id value 4 - ##TODO write a method in project model which return learning projects id - self.log_entries.find_by_project_id(4).update_attributes(log_text: learning) + learning_project_id = Project.where(name: 'Learning').pluck(:id).first.to_i + self.log_entries.find_by_project_id(learning_project_id).update_attributes(log_text: learning) end - #TODO add empty line after each method - def self.build_query_hash(params,current_user_id) + + def self.build_query_hash(params, current_user_id) query_hash ={} query_hash[:user_id] = params[:user_id] || current_user_id - #TODO refactor below code - if(!params[:start_date].nil?) + if(params[:start_date].present?) query_hash.merge!(log_date: params[:start_date]..params[:end_date]) else - query_hash.merge!(log_date: 1.week.ago..Date.today) + query_hash.merge!(log_date: 1.week.ago.strftime('%v')..Date.today.strftime('%v')) end query_hash end + end diff --git a/app/views/daily_logs/_user_summary.html.haml b/app/views/daily_logs/_user_summary.html.haml index feab829..2d4ce9d 100644 --- a/app/views/daily_logs/_user_summary.html.haml +++ b/app/views/daily_logs/_user_summary.html.haml @@ -11,13 +11,14 @@ =value[:logdate] %h3 Work - -value[:beautifulcode].each do |project,logtexts| - %a.ui.tag.label - =project - %ul - -logtexts.each do |logtext| - %li - =logtext[0] + -if value[:beautifulcode].present? + -value[:beautifulcode].each do |project,logtexts| + %a.ui.tag.label + =project + %ul + -logtexts.each do |logtext| + %li + =logtext[0] -value[:clients].each do |client,records| -records.each do |pname,logs| %a.ui.tag.label @@ -39,7 +40,7 @@ =value[:takeaway] -if(current_user.id == value[:user_id]) %div - =link_to "Edit log",edit_log_path(:log_id => value[:log_id]),:class => "btn btn-change btn-md pull-right" + =link_to "Edit log",edit_log_path(:daily_log_id => value[:log_id]),:class => "btn btn-change btn-md pull-right" %br %br %br diff --git a/app/views/daily_logs/edit_log.html.haml b/app/views/daily_logs/edit_log.html.haml index ba253e7..edeee7d 100644 --- a/app/views/daily_logs/edit_log.html.haml +++ b/app/views/daily_logs/edit_log.html.haml @@ -12,20 +12,21 @@ =@edit_log[:logdate] %h3.headerclass Work - -@edit_log[:beautifulcode].each do |project,logtexts| - %div - %div - =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :value => @projects.where(:name =>"#{project}").pluck(:id).first.to_s,:class => "added" - %h5.task - Task + -if @edit_log[:beautifulcode].present? + -@edit_log[:beautifulcode].each do |project,logtexts| %div %div - -logtexts.each do |logtext| - =text_field_tag("log_entries["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") - %br - %button.btn.btn-default.add_field_button - Add Task - %br + =select_tag "projects[]", options_for_select(@projects.map {|u| [u.name,u.id]}),:onchange => "alert_me_test()",:include_blank => "#{project}", :value => @projects.where(:name =>"#{project}").pluck(:id).first.to_s,:class => "added" + %h5.task + Task + %div + %div + -logtexts.each do |logtext| + =text_field_tag("log_entries["+@projects.where(:name =>"#{project}").pluck(:id).first.to_s+"]["+logtext[1].to_s+"]","#{logtext[0]}",:class => "form-control existing") + %br + %button.btn.btn-default.add_field_button + Add Task + %br %div.project -@edit_log[:clients].each do |client,records| -records.each do |project,logtexts| diff --git a/config/projects.yml b/config/projects.yml new file mode 100644 index 0000000..d1a21ff --- /dev/null +++ b/config/projects.yml @@ -0,0 +1,13 @@ +projects: + - name: internal_apps + client_name: BeautifulCode + - name: RevDirect + client_name: Sojern + - name: Debtinator + client_name: Sojern + - name: Learning + client_name: Learning + - name: Navigator + client_name: Sojern + + diff --git a/db/seeds.rb b/db/seeds.rb index 1beea2a..e5e1af3 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,7 +1,6 @@ -# This file should contain all the record creation needed to seed the database with its default values. -# The data can then be loaded with the rails db:seed command (or created alongside the database with db:setup). -# -# Examples: -# -# movies = Movie.create([{ name: 'Star Wars' }, { name: 'Lord of the Rings' }]) -# Character.create(name: 'Luke', movie: movies.first) +data = YAML.load_file 'config/projects.yml' +data['projects'].each do |project| + name = project["name"] + client_name = project["client_name"] + Project.create!(name: name, client_name: client_name) +end diff --git a/spec/factories.rb b/spec/factories.rb index 538b0d4..e93141b 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -6,7 +6,7 @@ provider "google" end factory :daily_log do - log_date '2017-05-31' + log_date Date.today takeaway 'spend more time on learning' user_id '1' end @@ -15,7 +15,7 @@ project_id "2" end factory :project do - name "revdirect" + name "macnator" client_name "Sojern" end factory :feedback do @@ -23,4 +23,5 @@ user_id "2" daily_log_id "2" end -end + end + diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index 45cbea3..a58161c 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -1,14 +1,33 @@ require 'rails_helper' RSpec.describe DailyLog, type: :model do - #pending "add some examples to (or delete) #{__FILE__}" let(:user) {User.create(name:"avinash",email:"avinash@beautifulcode.in") } let(:daily_log) { FactoryGirl.build(:daily_log) } let(:log_entry) { FactoryGirl.build(:log_entry) } + let(:project) { FactoryGirl.create(:project) } + let(:learning_project) { FactoryGirl.build(:project) } + let(:learning) { FactoryGirl.build(:log_entry) } + let(:revdirect) { FactoryGirl.build(:project) } + let(:revdirect_log_entry) { FactoryGirl.build(:log_entry) } before do daily_log.user_id=user.id daily_log.save log_entry.daily_log_id=daily_log.id + log_entry.project_id=project.id log_entry.save + learning_project.name="Learning" + learning_project.client_name="Learning" + learning_project.save + learning.daily_log_id=daily_log.id + learning.project_id=learning_project.id + learning.log_text ="advanced jquery" + learning.save + revdirect.name="revdirect" + revdirect.client_name="Sojern" + revdirect.save + revdirect_log_entry.log_text ="solved ticket 3" + revdirect_log_entry.project_id =revdirect.id + revdirect_log_entry.daily_log_id=daily_log.id + revdirect_log_entry.save end [:log_date, :takeaway].each do |msg| @@ -19,36 +38,34 @@ it "should have many log_entries" do association = DailyLog.reflect_on_association(:log_entries) - association.macro.should == :has_many expect(association.macro).to eq(:has_many) end it 'should belong to a user' do association = DailyLog.reflect_on_association(:user) - #TODO refactor this - association.macro.should == :belongs_to + + expect(association.macro).to eq(:belongs_to) end - it 'should have only one dailylog entry for given date for given user' do + it 'should have one or none dailylog entry for given date for given user' do @dailylog = DailyLog.where(log_date: Date.today,user_id: 1) - #expect(@dailylog.count). to be <= 1 - expect(@dailylog.count).to eq(1) + expect(@dailylog.count).to be <= 1 end it 'should delete all associated log entries on deleting daily log' do - #count = DailyLog.first.log_entries.count - #total_count = LogEntry.all.count - #DailyLog.first.destroy - #expect(LogEntry.all.count).to equal(total_count - count) - #count =daily_log.log_entries.count - count=LogEntry.all.count - expect(count).to eq(1) + count =daily_log.log_entries.count + total_count =LogEntry.all.count + daily_log.destroy + + expect(LogEntry.all.count).to eq(total_count-count) end + describe 'validations' do it 'should have valid user_id' do daily_log.update_attributes(user_id: nil) + expect(daily_log.errors).to include(:user_id) end @@ -56,38 +73,74 @@ describe :user_create_summary do context "when project id is nil" do - it "should return daily logs for als projects" do + it "should return daily logs for all projects" do + params= { user_id: user.id } + records = daily_log.class.create_user_summary(params,nil,nil,nil) + expect(records.values.first[:clients].values.first.keys).to match_array(["macnator","revdirect"]) end end - context "when project id is not nil" do + it "should return daily_logs for specific project" do + params ={user_id: user.id} + records= daily_log.class.create_user_summary(params,nil,revdirect.id,nil) + expect(records.values.first[:clients].values.first.keys).to match_array(["revdirect"]) + end end - context "when project_id and query_hash are not present and log date is present" do + it "should return daily logs of all users for that particular date" do + records= daily_log.class.create_user_summary(nil,nil,nil,Date.today) + expect(records.values.first[:logdate]).to eq(Date.today.strftime('%v')) + end end - end - describe :create_record do - log=DailyLog.new - query_hash={user_id: 1,log_date:1.week.ago..Date.today} + describe :create_summary_record do before do - log.id=1 - log.user_id = 1 - log.log_date=Date.today - log.takeaway="need to improve" - log.save - allow(log).to receive(:beautifulcode_log_record).and_return nil - allow(log).to receive(:clients_project_log_record).and_return nil + allow(daily_log).to receive(:clients_project_log_record).and_return nil end it 'should return formatted daily log record' do - #:log_date = Date.today - record = log.class.user_create_summary(query_hash,nil,nil) - expect(record.keys).to match_array([:name, :takeaway, :learning, :beautifulcode, :clients]) - expect(record.keys).to match_array([:name]) + record = daily_log.create_summary_record(nil) + + expect(record.keys).to match_array([:name, :takeaway, :learning,:clients,:log_id,:logdate,:picture,:user_id]) + end + end + describe :build_query_hash do + context "when date filter applied" do + it "should return valid hash" do + params = { user_id: 1, start_date: '25-07-2017',end_date: '30-07-2017' } + query_hash = daily_log.class.build_query_hash(params,nil) + + expect(query_hash.keys).to match_array([:user_id,:log_date]) + expect(query_hash.values[1]).to eq("25-07-2017".."30-07-2017") + end + end + context "when date filter not applied" do + it "should return valid hash" do + params={ user_id: 1 } + query_hash= daily_log.class.build_query_hash(params,nil) + value= 1.week.ago.strftime('%v')..Date.today.strftime('%v') + + expect(query_hash.keys).to match_array([:user_id,:log_date]) + expect(query_hash.values[1]).to eq(value) + end + end + end + + describe :clients_project_log_record do + context "when project filter not applied" do + it "should return all project log entries" do + record =daily_log.clients_project_log_record(nil) + expect(record.values.first.keys).to match_array(["macnator","revdirect"]) + end + end + context "when project filter applied" do + it "should return project specific log entries" do + record =daily_log.clients_project_log_record(revdirect.id) + expect(record.values.first.keys).to match_array(["revdirect"]) + end end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f8c3425..0e6eec1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,12 +1,7 @@ require 'rails_helper' RSpec.describe Project, type: :model do - #<<<<<<< HEAD - #let(:project) { FactoryGirl.build(:Project) } - #let(:id) { '111111' } - #[:name, :client_name].each do |msg| - #======= - let(:project) { FactoryGirl.build(:project) } + let(:project) { FactoryGirl.create(:project) } [:name, :client_name].each do |msg| it "should respond to #{msg}" do expect(project).to respond_to(msg) @@ -23,15 +18,15 @@ end #context 'when newly created' do #it 'should not have any log entries' do - #LogEntry.joins(:project).where("projects.id=1111").count.should == 0 + #count = LogEntry.joins(:project).where("projects.id=#{project.id}").count + #expect(count).to eq(0) #end #it 'should not have any users' do - #User.joins(:project).where("projects.id=1111").count.should == 0 + #count = User.joins(:project).where("projects.id=#{project.id}").count + #expect(count).to eq(0) #end #end - - #it 'should not have any users' end From d303ba5d1e375fc0b88d86882e71b6d5dc71c614 Mon Sep 17 00:00:00 2001 From: saichander Date: Sat, 1 Jul 2017 14:33:05 +0530 Subject: [PATCH 11/17] code review comments 1st JULY --- app/assets/javascripts/daily_log.js | 2 ++ app/assets/stylesheets/application.scss | 1 + app/controllers/application_controller.rb | 5 ++++- app/controllers/daily_logs_controller.rb | 10 ++++++++++ app/models/daily_log.rb | 19 +++++++++++++++---- app/models/log_entry.rb | 1 + db/seeds.rb | 8 +++++--- spec/models/dailylog_spec.rb | 10 ++++++++++ spec/models/feedback_spec.rb | 1 + spec/models/logentry_spec.rb | 10 +++++++--- spec/models/project_spec.rb | 1 + spec/models/user_spec.rb | 3 +++ 12 files changed, 60 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/daily_log.js b/app/assets/javascripts/daily_log.js index 00096b8..30f5aa7 100644 --- a/app/assets/javascripts/daily_log.js +++ b/app/assets/javascripts/daily_log.js @@ -48,6 +48,7 @@ function DailyLog() { maxDate: new Date(today.getFullYear(), today.getMonth(), today.getDate()) }) $('.top.menu .item').tab({'onVisible':function(){ + // TODO dont you need to put semi colon at EOL var newValue = $(this).attr("dataval") $('.dynamic').load('/daily_logs/refresh',{ "log_date":newValue}); }}); @@ -57,6 +58,7 @@ function DailyLog() { }); $('.secondary.menu .item').tab({'onVisible':function(){ + // TODO use proper variable names var x=$(this).attr('data-tab'); if(x=="first") location.reload(); diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 8ba547f..2ccf76c 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,3 +1,4 @@ +// TODO you should put css related different pages in different files // Custom bootstrap variables must be set or imported *before* bootstrap. @import "bootstrap"; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3634031..f6c3544 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ApplicationController < ActionController::Base - #TODO add authencticate_user in before action + #TODO add authenticate_user in before action protect_from_forgery with: :exception helper_method :current_user @@ -9,9 +9,12 @@ class ApplicationController < ActionController::Base def current_user @current_user ||= User.find(session[:user_id]) if session[:user_id] end + #TODO add empty line after evry action + #TODO add authenticate_user in before_action for ApplicationController def authenticate_user if (current_user.nil?) url="/auth/google_oauth2" + #TODO add flash message "please login to continue" redirect_to(url) end end diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index c7d5d94..6530f30 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -1,15 +1,22 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token + #TODO change the flow, so that you even need to authenticate_user on index action before_action :authenticate_user, :except=>[:index] def new @daily_log = current_user.daily_logs.new + #TODO dont hard code learning project @projects = Project.where("name != 'Learning'") + #TODO use space after comma, prefer named arguments, last parameter you are sending is nil but I dont know which + # value you are sending nil, if you args looks like this DailyLog.create_user_summary(something: nil) + # it would be better @user_log_summary = DailyLog.create_user_summary(params,current_user.id,params[:project_id],nil) end def create + #TODO move this logic to model it should return flags respectively, so that you can set flash messages here + # after moving this to a method write specs for it if(DailyLog.where(user_id: current_user.id,log_date:params[:datetime_ida]).count==0) begin ActiveRecord::Base.transaction do @@ -52,11 +59,14 @@ def user_logs def edit_log @edit_log = DailyLog.find(params[:daily_log_id]).create_summary_record(nil) + #TODO dont hard code values @projects = Project.where("name != 'Learning'") @daily_log_id = params[:daily_log_id] end def update + #TODO move this logic to model method, change update_daily_log_and_log_entries to + # to do all this logic begin ActiveRecord::Base.transaction do DailyLog.find(params[:daily_log_id]).update_daily_log_and_log_entries(params[:log_entries],params[:learning_log],params[:takeaway]) diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 7bb61c4..7cadb68 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -2,17 +2,24 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry', :dependent => :delete_all belongs_to :user validates :user_id, presence: true + #TODO add a custom validation so that for a given user on a given log_date there should not be more than one record + ##TODO you have logic to restrict this in create action but restriction at model level would be better option include ApplicationHelper def self.create_user_summary(params,user_id, project_id, log_date) - if(params.present?) - query_hash = DailyLog.build_query_hash(params,user_id) - end + #TODO use single line if statements when line length is small + #if(params.present?) + # query_hash = DailyLog.build_query_hash(params,user_id) + #end + query_hash = DailyLog.build_query_hash(params,user_id) if params.present? if( project_id.present? ) + #TODO query_hash will be undefined when params.present? will return false so it raises error daily_logs = DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct else daily_logs = DailyLog.where(query_hash).order("log_date DESC") end + #TODO why are you doing query_hash.nil? it always returns false or error when params.present? is false + ##TODO refactor this if condition and above if condition, use if elsif elsif end if(query_hash.nil? and project_id.nil? and !log_date.nil? ) daily_logs = DailyLog.where(log_date: log_date).order("log_date DESC") end @@ -31,6 +38,7 @@ def create_summary_record(project_id) summary_record[:log_id] = self.id summary_record[:takeaway] = self.takeaway.to_s #TODO dont hard code client name values + #TODO why are not using Project.where(client_name: 'Learning').first.id summary_record[:learning]= self.log_entries.where(project_id:Project.where(client_name:'Learning')).first.log_text # summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text summary_record[:clients] = self.clients_project_log_record(project_id) @@ -50,9 +58,11 @@ def clients_project_log_record(project_id) end project_log_logtext = Hash.new{|h,k| h[k] = []} project_log_ids.each do |project_id_log_id| + #TODO how many should I tell you to use find, that would be enough + ##TODO add space before/after = << or any other operator project_log_logtext[Project.find_by_id(project_id_log_id[1]).name] <<[LogEntry.find_by_id(project_id_log_id[0]).log_text,project_id_log_id[0]] end - project_logs[client.to_s]=project_log_logtext; + project_logs[client.to_s]=project_log_logtext end project_logs end @@ -64,6 +74,7 @@ def update_daily_log_and_log_entries(log_entry_records,learning,takeaway) end end self.update_attributes(:takeaway => takeaway) + #TODO dnt hard code values learning_project_id = Project.where(name: 'Learning').pluck(:id).first.to_i self.log_entries.find_by_project_id(learning_project_id).update_attributes(log_text: learning) end diff --git a/app/models/log_entry.rb b/app/models/log_entry.rb index 9cc9285..c1e3834 100644 --- a/app/models/log_entry.rb +++ b/app/models/log_entry.rb @@ -3,6 +3,7 @@ class LogEntry < ApplicationRecord belongs_to :project #def create_logentry(project_id,logtext,daily_log_id) + ##TODO wheh you are not using a method remove it def create_logentry(project_id,logtext) LogEntry.create(project_id: project_id,log_text: logtext,daily_log_id: daily_log_id) end diff --git a/db/seeds.rb b/db/seeds.rb index e5e1af3..61ca0cd 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,6 +1,8 @@ data = YAML.load_file 'config/projects.yml' data['projects'].each do |project| - name = project["name"] - client_name = project["client_name"] - Project.create!(name: name, client_name: client_name) + #TODO refactor this + #name = project["name"] + #client_name = project["client_name"] + #Project.create!(name: name, client_name: client_name) + Project.create!(name: project["name"], client_name: project["client_name"]) end diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index a58161c..a9d197f 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -14,6 +14,7 @@ log_entry.daily_log_id=daily_log.id log_entry.project_id=project.id log_entry.save + #TODO learning_project = Project.create(name: "Learning", client_name: "Learning") learning_project.name="Learning" learning_project.client_name="Learning" learning_project.save @@ -21,6 +22,7 @@ learning.project_id=learning_project.id learning.log_text ="advanced jquery" learning.save + #TODO chnage this same as above one revdirect.name="revdirect" revdirect.client_name="Sojern" revdirect.save @@ -59,6 +61,7 @@ total_count =LogEntry.all.count daily_log.destroy + #TODO instead you can check expect(daily_log.log_entries.count).to eq(0) expect(LogEntry.all.count).to eq(total_count-count) end @@ -68,10 +71,13 @@ expect(daily_log.errors).to include(:user_id) end + #TODO when you add custom validation to restrict one daily_log record per user per given date add specs for it end + #TODO it should be create_user_summary describe :user_create_summary do + #TODO when params.present? if false write case for it context "when project id is nil" do it "should return daily logs for all projects" do params= { user_id: user.id } @@ -99,6 +105,7 @@ describe :create_summary_record do before do + #TODO instead of returning nil return what sample data of what that method actually should return allow(daily_log).to receive(:clients_project_log_record).and_return nil end it 'should return formatted daily log record' do @@ -107,6 +114,8 @@ expect(record.keys).to match_array([:name, :takeaway, :learning,:clients,:log_id,:logdate,:picture,:user_id]) end end + #TODO add empty line after every block + ##TODO write specs for methods in the same order as written in model file describe :build_query_hash do context "when date filter applied" do it "should return valid hash" do @@ -142,6 +151,7 @@ expect(record.values.first.keys).to match_array(["revdirect"]) end end + #TODO add a case to test that record.values.first.keys not to include "learning" end end diff --git a/spec/models/feedback_spec.rb b/spec/models/feedback_spec.rb index 9b24bf1..a1fbfb4 100644 --- a/spec/models/feedback_spec.rb +++ b/spec/models/feedback_spec.rb @@ -1,6 +1,7 @@ require 'rails_helper' RSpec.describe Feedback, type: :model do + #TODO change feedbackentry feedback_entry let(:feedbackentry) { FactoryGirl.build(:feedback) } [:feedback_text, :user_id,:daily_log_id].each do |msg| it "should respond to #{msg}" do diff --git a/spec/models/logentry_spec.rb b/spec/models/logentry_spec.rb index 3c2dc17..31ec7e3 100644 --- a/spec/models/logentry_spec.rb +++ b/spec/models/logentry_spec.rb @@ -8,15 +8,19 @@ end end - it "should belongs to daily_log" do + it "should belong to daily_log" do association = LogEntry.reflect_on_association(:daily_log) - association.macro.should == :belongs_to + #TODO reafctor all expect statements like below one + #association.macro.should == :belongs_to + expect(association.macro).to eq(:belongs_to) end - it "should belongs to project" do + #TODO add empty line each test case + it "should belong to project" do association = LogEntry.reflect_on_association(:project) association.macro.should == :belongs_to end it "should have valid log_text" do + #TODO fill this end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 0e6eec1..b61db7d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -16,6 +16,7 @@ association = Project.reflect_on_association(:log_entries) association.macro.should == :has_many end + #TODO why did you comment below code, complete it #context 'when newly created' do #it 'should not have any log entries' do #count = LogEntry.joins(:project).where("projects.id=#{project.id}").count diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 489264c..dc3631c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -25,10 +25,12 @@ expect(user.email).to eq('dummy@dummy.com') expect(user.oauth_token).to eq('dummy') #expect(user.token_expires_at).to eq(123_456) + ##TODO remove unneccessary empty lines end end + #TODO add empty line after each test case it "should have many daily_logs" do association = User.reflect_on_association(:daily_logs) association.macro.should == :has_many @@ -60,6 +62,7 @@ context 'when authentication fails' do + #TODO complete this behaviour # describe what happens when authentication fails. end From 3d950517f2d649f8266ee61e7b94c45dba88c3bf Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 1 Jul 2017 21:22:05 +0530 Subject: [PATCH 12/17] model specs and bug fix --- app/assets/javascripts/edit_log.js | 2 +- app/controllers/daily_logs_controller.rb | 28 ++--- app/models/daily_log.rb | 47 ++++----- app/models/project.rb | 3 +- spec/models/dailylog_spec.rb | 126 ++++++++++++++++------- spec/models/feedback_spec.rb | 7 +- spec/models/logentry_spec.rb | 7 +- spec/models/project_spec.rb | 20 ++-- spec/models/user_spec.rb | 24 +---- 9 files changed, 131 insertions(+), 133 deletions(-) diff --git a/app/assets/javascripts/edit_log.js b/app/assets/javascripts/edit_log.js index 393d591..0b666a9 100644 --- a/app/assets/javascripts/edit_log.js +++ b/app/assets/javascripts/edit_log.js @@ -19,7 +19,7 @@ function EditLog() { $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.NewlyAdded').attr('name',name); $(this).parent().parent().children().eq(2).children().eq(0).children().filter('.existing').each(function(i){ var oldName= $(this).attr('name'); - var newName='Logtext['+project_id+']'+oldName.substring(oldName.lastIndexOf('[')); + var newName='log_entries['+project_id+']'+oldName.substring(oldName.lastIndexOf('[')); $(this).attr('name',newName); }); }); diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index 6530f30..3189c96 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -6,18 +6,12 @@ class DailyLogsController < ApplicationController def new @daily_log = current_user.daily_logs.new - #TODO dont hard code learning project - @projects = Project.where("name != 'Learning'") - #TODO use space after comma, prefer named arguments, last parameter you are sending is nil but I dont know which - # value you are sending nil, if you args looks like this DailyLog.create_user_summary(something: nil) - # it would be better - @user_log_summary = DailyLog.create_user_summary(params,current_user.id,params[:project_id],nil) + @projects = Project.where("name !='"+Project::LEARNING+"'") + @user_log_summary = DailyLog.create_user_summary(params,current_user.id, params[:project_id],log_date:nil) end def create - #TODO move this logic to model it should return flags respectively, so that you can set flash messages here - # after moving this to a method write specs for it - if(DailyLog.where(user_id: current_user.id,log_date:params[:datetime_ida]).count==0) + if(DailyLog.check_log_entry_exists(current_user.id,params[:datetime_ida])) begin ActiveRecord::Base.transaction do daily_log = current_user.daily_logs.create(takeaway: params[:takeaway],log_date: params[:datetime_ida]) @@ -43,7 +37,7 @@ def index end def people_logs - @log_summary = DailyLog.create_user_summary(nil,nil,nil,Date.today) + @log_summary = DailyLog.create_user_summary(nil, nil, nil,Date.today) @all_users = User.all; end @@ -59,24 +53,14 @@ def user_logs def edit_log @edit_log = DailyLog.find(params[:daily_log_id]).create_summary_record(nil) - #TODO dont hard code values - @projects = Project.where("name != 'Learning'") + @projects = Project.where("name !='"+Project::LEARNING+"'") @daily_log_id = params[:daily_log_id] end def update - #TODO move this logic to model method, change update_daily_log_and_log_entries to - # to do all this logic begin ActiveRecord::Base.transaction do - DailyLog.find(params[:daily_log_id]).update_daily_log_and_log_entries(params[:log_entries],params[:learning_log],params[:takeaway]) - if(!params[:new_log_entries].nil?) - params[:new_log_entries].each do |project_id,log_entries| - log_entries.each do|logtext| - DailyLog.find(params[:daily_log_id]).log_entries.create(project_id:project_id,log_text:logtext) - end - end - end + DailyLog.find(params[:daily_log_id]).update_daily_log_and_log_entries(params[:log_entries],params[:learning_log],params[:takeaway],params[:new_log_entries]) end flash[:info] = "Updated successfully" redirect_to new_daily_log_path diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 7cadb68..4acb5db 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -2,27 +2,18 @@ class DailyLog < ApplicationRecord has_many :log_entries, :class_name => 'LogEntry', :dependent => :delete_all belongs_to :user validates :user_id, presence: true - #TODO add a custom validation so that for a given user on a given log_date there should not be more than one record - ##TODO you have logic to restrict this in create action but restriction at model level would be better option include ApplicationHelper def self.create_user_summary(params,user_id, project_id, log_date) - #TODO use single line if statements when line length is small - #if(params.present?) - # query_hash = DailyLog.build_query_hash(params,user_id) - #end + query_hash = {} query_hash = DailyLog.build_query_hash(params,user_id) if params.present? if( project_id.present? ) - #TODO query_hash will be undefined when params.present? will return false so it raises error daily_logs = DailyLog.where(query_hash).joins(:log_entries).where(log_entries: {project_id: project_id}).order("log_date DESC").distinct + elsif(!query_hash.present? and project_id.nil? and log_date.present? ) + daily_logs = DailyLog.where(log_date: log_date).order("log_date DESC") else daily_logs = DailyLog.where(query_hash).order("log_date DESC") end - #TODO why are you doing query_hash.nil? it always returns false or error when params.present? is false - ##TODO refactor this if condition and above if condition, use if elsif elsif end - if(query_hash.nil? and project_id.nil? and !log_date.nil? ) - daily_logs = DailyLog.where(log_date: log_date).order("log_date DESC") - end daily_log_summary = Hash.new daily_logs.each_with_index do |daily_log,index| daily_log_summary[index] = daily_log.create_summary_record(project_id) @@ -37,18 +28,14 @@ def create_summary_record(project_id) summary_record[:picture] = self.user.picture summary_record[:log_id] = self.id summary_record[:takeaway] = self.takeaway.to_s - #TODO dont hard code client name values - #TODO why are not using Project.where(client_name: 'Learning').first.id - summary_record[:learning]= self.log_entries.where(project_id:Project.where(client_name:'Learning')).first.log_text - # summary_record[:learning] = self.log_entries.where(project_id: Project.where(client_name: 'Learning').first.id).first.log_text + summary_record[:learning]= self.log_entries.where(project_id:Project.where(client_name: Project::LEARNING).first.id).first.log_text summary_record[:clients] = self.clients_project_log_record(project_id) summary_record[:logdate] = self.log_date.strftime('%v') summary_record end def clients_project_log_record(project_id) - #TODO dont hard code client names - clients = Project.where.not(client_name: 'Learning').distinct.pluck('client_name') + clients = Project.where.not(client_name: Project::LEARNING).distinct.pluck('client_name') project_logs = Hash.new() clients.each do |client| if( project_id.present? ) @@ -58,25 +45,29 @@ def clients_project_log_record(project_id) end project_log_logtext = Hash.new{|h,k| h[k] = []} project_log_ids.each do |project_id_log_id| - #TODO how many should I tell you to use find, that would be enough - ##TODO add space before/after = << or any other operator - project_log_logtext[Project.find_by_id(project_id_log_id[1]).name] <<[LogEntry.find_by_id(project_id_log_id[0]).log_text,project_id_log_id[0]] + project_log_logtext[Project.find(project_id_log_id[1]).name] << [LogEntry.find_by_id(project_id_log_id[0]).log_text,project_id_log_id[0]] end project_logs[client.to_s]=project_log_logtext end project_logs end - def update_daily_log_and_log_entries(log_entry_records,learning,takeaway) + def update_daily_log_and_log_entries(log_entry_records,learning,takeaway,new_log_entries) log_entry_records.each do |project_id,log_entries| log_entries.each do |log_entry_id,log_text| self.log_entries.find(log_entry_id).update_attributes(project_id: project_id, log_text:log_text); end end self.update_attributes(:takeaway => takeaway) - #TODO dnt hard code values - learning_project_id = Project.where(name: 'Learning').pluck(:id).first.to_i + learning_project_id = Project.where(name: Project::LEARNING).pluck(:id).first.to_i self.log_entries.find_by_project_id(learning_project_id).update_attributes(log_text: learning) + if(new_log_entries.present?) + new_log_entries.each do |project_id,log_entries| + log_entries.each do|logtext| + self.log_entries.create(project_id:project_id,log_text:logtext) + end + end + end end def self.build_query_hash(params, current_user_id) @@ -89,5 +80,11 @@ def self.build_query_hash(params, current_user_id) end query_hash end - + def self.check_log_entry_exists(user_id,log_date) + if(DailyLog.where(user_id: user_id, log_date:log_date).count==0) + true + else + false + end + end end diff --git a/app/models/project.rb b/app/models/project.rb index b379b2d..642f521 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1,9 +1,8 @@ class Project < ApplicationRecord - #attr_accessible :name, :client has_and_belongs_to_many :users has_many :log_entries , :class_name => 'LogEntry' accepts_nested_attributes_for :log_entries - + BEAUTIFUL_CODE_CLIENT = "BeautifulCode" SOJERN_CLIENT = "Sojern" LEARNING = "Learning" diff --git a/spec/models/dailylog_spec.rb b/spec/models/dailylog_spec.rb index a9d197f..2a3ddb2 100644 --- a/spec/models/dailylog_spec.rb +++ b/spec/models/dailylog_spec.rb @@ -4,31 +4,21 @@ let(:daily_log) { FactoryGirl.build(:daily_log) } let(:log_entry) { FactoryGirl.build(:log_entry) } let(:project) { FactoryGirl.create(:project) } - let(:learning_project) { FactoryGirl.build(:project) } - let(:learning) { FactoryGirl.build(:log_entry) } - let(:revdirect) { FactoryGirl.build(:project) } - let(:revdirect_log_entry) { FactoryGirl.build(:log_entry) } + let(:learning_project) { Project.create(name: "Learning", client_name: "Learning") } + let(:learning) { LogEntry.new } + let(:revdirect) { Project.create(name:"revdirect", client_name:"Sojern") } + let(:revdirect_log_entry) { LogEntry.new } before do + Project.delete_all + LogEntry.delete_all daily_log.user_id=user.id daily_log.save log_entry.daily_log_id=daily_log.id log_entry.project_id=project.id log_entry.save - #TODO learning_project = Project.create(name: "Learning", client_name: "Learning") - learning_project.name="Learning" - learning_project.client_name="Learning" - learning_project.save - learning.daily_log_id=daily_log.id - learning.project_id=learning_project.id - learning.log_text ="advanced jquery" + learning.update_attributes(daily_log_id: daily_log.id,project_id: learning_project.id,log_text: "advanced jquery") learning.save - #TODO chnage this same as above one - revdirect.name="revdirect" - revdirect.client_name="Sojern" - revdirect.save - revdirect_log_entry.log_text ="solved ticket 3" - revdirect_log_entry.project_id =revdirect.id - revdirect_log_entry.daily_log_id=daily_log.id + revdirect_log_entry.update_attributes(log_text:"Solved ticket 3", project_id: revdirect.id , daily_log_id: daily_log.id) revdirect_log_entry.save end @@ -61,7 +51,6 @@ total_count =LogEntry.all.count daily_log.destroy - #TODO instead you can check expect(daily_log.log_entries.count).to eq(0) expect(LogEntry.all.count).to eq(total_count-count) end @@ -71,13 +60,9 @@ expect(daily_log.errors).to include(:user_id) end - #TODO when you add custom validation to restrict one daily_log record per user per given date add specs for it - end - #TODO it should be create_user_summary - describe :user_create_summary do - #TODO when params.present? if false write case for it + describe :create_user_summary do context "when project id is nil" do it "should return daily logs for all projects" do params= { user_id: user.id } @@ -95,17 +80,16 @@ end end context "when project_id and query_hash are not present and log date is present" do - it "should return daily logs of all users for that particular date" do - records= daily_log.class.create_user_summary(nil,nil,nil,Date.today) + it "should return daily logs of all users for that particular date" do + records= daily_log.class.create_user_summary(nil,nil,nil,Date.today) expect(records.values.first[:logdate]).to eq(Date.today.strftime('%v')) - end + end end end describe :create_summary_record do before do - #TODO instead of returning nil return what sample data of what that method actually should return allow(daily_log).to receive(:clients_project_log_record).and_return nil end it 'should return formatted daily log record' do @@ -114,8 +98,71 @@ expect(record.keys).to match_array([:name, :takeaway, :learning,:clients,:log_id,:logdate,:picture,:user_id]) end end - #TODO add empty line after every block - ##TODO write specs for methods in the same order as written in model file + + describe :clients_project_log_record do + context "when project filter not applied" do + it "should return all project log entries" do + record = daily_log.clients_project_log_record(nil) + expect(record.values.first.keys).to match_array(["macnator","revdirect"]) + end + end + context "when project filter applied" do + it "should return project specific log entries" do + record =daily_log.clients_project_log_record(revdirect.id) + expect(record.values.first.keys).to match_array(["revdirect"]) + end + end + it "should not return learning log entries" do + record = daily_log.clients_project_log_record(nil) + project_record = daily_log.clients_project_log_record(revdirect.id) + + expect(record.values.first.keys).to match_array(["macnator","revdirect"]) + expect(project_record.values.first.keys).to match_array(["revdirect"]) + + end + end + + describe :update_daily_log_and_log_entries do + context "when new log entries added for dailylog" do + it "should add those logentries to database" do + log_entries = {revdirect.id => {revdirect_log_entry.id => "edited"}} + new_log_entries = { revdirect.id =>["new_record"] } + takeaway = "checing_updation" + learning = "checking_updation" + before_count = daily_log.log_entries.count + daily_log.update_daily_log_and_log_entries(log_entries,learning,takeaway,new_log_entries) + after_count =daily_log.log_entries.count + + expect(after_count).to eq(before_count+1) + + end + end + context "when existing records are modified" do + it "should update those logentries in database" do + log_entries = {revdirect.id => {revdirect_log_entry.id => "edited"}} + takeaway = "checking_updation" + learning = "checking_updation" + daily_log.update_daily_log_and_log_entries(log_entries,learning,takeaway,nil) + + expect(daily_log.log_entries.find(revdirect_log_entry.id).log_text).to eq("edited") + expect(daily_log.takeaway).to eq("checking_updation") + end + end + context "when no new log_entries are added for daily_log" do + it "should not add any log entries in database for that daily_log" do + log_entries = {revdirect.id => {revdirect_log_entry.id => "edited"}} + takeaway = "checking_updation" + learning = "checking_updation" + before_count =daily_log.log_entries.count + daily_log.update_daily_log_and_log_entries(log_entries,learning,takeaway,nil) + updated_count = daily_log.log_entries.count + + expect(updated_count).to eq(before_count) + end + end + end + + describe :build_query_hash do context "when date filter applied" do it "should return valid hash" do @@ -138,20 +185,19 @@ end end - describe :clients_project_log_record do - context "when project filter not applied" do - it "should return all project log entries" do - record =daily_log.clients_project_log_record(nil) - expect(record.values.first.keys).to match_array(["macnator","revdirect"]) + describe :check_log_entry_exists do + context "when log entry for the day exists" do + it "should return false " do + flag = daily_log.class.check_log_entry_exists(user.id, Date.today) + expect(flag).to eq(false) end end - context "when project filter applied" do - it "should return project specific log entries" do - record =daily_log.clients_project_log_record(revdirect.id) - expect(record.values.first.keys).to match_array(["revdirect"]) + context "when log entry for the day does not exists" do + it "should return true" do + flag = daily_log.class.check_log_entry_exists(user.id, Date.yesterday) + expect(flag).to eq(true) end end - #TODO add a case to test that record.values.first.keys not to include "learning" end end diff --git a/spec/models/feedback_spec.rb b/spec/models/feedback_spec.rb index a1fbfb4..410215d 100644 --- a/spec/models/feedback_spec.rb +++ b/spec/models/feedback_spec.rb @@ -1,11 +1,10 @@ require 'rails_helper' RSpec.describe Feedback, type: :model do - #TODO change feedbackentry feedback_entry - let(:feedbackentry) { FactoryGirl.build(:feedback) } - [:feedback_text, :user_id,:daily_log_id].each do |msg| + let(:feedback_entry) { FactoryGirl.build(:feedback) } + [:feedback_text, :user_id,:daily_log_id].each do |msg| it "should respond to #{msg}" do - expect(feedbackentry).to respond_to(msg) + expect(feedback_entry).to respond_to(msg) end end it "should belongs to user" do diff --git a/spec/models/logentry_spec.rb b/spec/models/logentry_spec.rb index 31ec7e3..45e1782 100644 --- a/spec/models/logentry_spec.rb +++ b/spec/models/logentry_spec.rb @@ -10,17 +10,14 @@ it "should belong to daily_log" do association = LogEntry.reflect_on_association(:daily_log) - #TODO reafctor all expect statements like below one - #association.macro.should == :belongs_to expect(association.macro).to eq(:belongs_to) end - #TODO add empty line each test case it "should belong to project" do association = LogEntry.reflect_on_association(:project) - association.macro.should == :belongs_to + expect(association.macro).to eq(:belongs_to) end it "should have valid log_text" do - #TODO fill this + expect(entry.log_text.to_s.length).to be > 0 end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b61db7d..c433bc4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe Project, type: :model do - let(:project) { FactoryGirl.create(:project) } + let(:project) { FactoryGirl.create(:project) } [:name, :client_name].each do |msg| it "should respond to #{msg}" do expect(project).to respond_to(msg) @@ -16,17 +16,13 @@ association = Project.reflect_on_association(:log_entries) association.macro.should == :has_many end - #TODO why did you comment below code, complete it - #context 'when newly created' do - #it 'should not have any log entries' do - #count = LogEntry.joins(:project).where("projects.id=#{project.id}").count - #expect(count).to eq(0) - #end - #it 'should not have any users' do - #count = User.joins(:project).where("projects.id=#{project.id}").count - #expect(count).to eq(0) - #end - #end + + context 'when newly created' do + it 'should not have any log entries' do + count = project.log_entries.count + expect(count).to eq(0) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dc3631c..2dbb6fb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -24,14 +24,10 @@ expect(user.name).to eq('dummy') expect(user.email).to eq('dummy@dummy.com') expect(user.oauth_token).to eq('dummy') - #expect(user.token_expires_at).to eq(123_456) - ##TODO remove unneccessary empty lines - - end end - #TODO add empty line after each test case - it "should have many daily_logs" do + + it "should have many daily_logs" do association = User.reflect_on_association(:daily_logs) association.macro.should == :has_many end @@ -50,22 +46,6 @@ expect(user.errors).to include(:email) end end - #context 'when newly created' do - #it 'should return an empty collection for projects' do - #User.joins(:project).where('id=?',user.id).count.should == 0 - #end - #it 'should return an empty collection for daily_logs' do - #User.joins(:daily_logs).where('id=?',user.id).count.should == 0 - #end - ##expect(User.count).to eq(before_user_count+1) - #end - - - context 'when authentication fails' do - #TODO complete this behaviour - # describe what happens when authentication fails. - end - end From 1a82563797c4c270ba26451216be3b86939543ed Mon Sep 17 00:00:00 2001 From: avinash patnala Date: Sat, 1 Jul 2017 21:24:06 +0530 Subject: [PATCH 13/17] Update README.md --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 8ba7620..5682481 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,9 @@ bundle exec rake db:create # To create the database bundle exec rake db:migrate # To run the migrations +bundle exec rake db:seed #to run the seed file: + bundle exec rails s # Start the rails server. + Open 'http://localhost:3000' in your browser. From 68b456524a8d54f164180d0e7443f17d09e307d3 Mon Sep 17 00:00:00 2001 From: avinash patnala Date: Sat, 1 Jul 2017 21:24:25 +0530 Subject: [PATCH 14/17] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5682481..d54f67a 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ bundle exec rake db:create # To create the database bundle exec rake db:migrate # To run the migrations -bundle exec rake db:seed #to run the seed file: +bundle exec rake db:seed #to run the seed file. bundle exec rails s # Start the rails server. From bd9ed91a3682c8ae0f8bb952839af7f7ae8d8ddf Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 1 Jul 2017 21:52:48 +0530 Subject: [PATCH 15/17] check_log_exists_method refactor --- app/models/daily_log.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index 4acb5db..fc5657e 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -81,10 +81,6 @@ def self.build_query_hash(params, current_user_id) query_hash end def self.check_log_entry_exists(user_id,log_date) - if(DailyLog.where(user_id: user_id, log_date:log_date).count==0) - true - else - false - end + flag = DailyLog.where(user_id: user_id, log_date:log_date).count==0 ? true : false end end From 5ac96ad9ec068cd450394f396b42ca7f94b1d207 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sat, 1 Jul 2017 21:56:16 +0530 Subject: [PATCH 16/17] removing comments --- app/controllers/daily_logs_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/daily_logs_controller.rb b/app/controllers/daily_logs_controller.rb index 3189c96..8f91261 100644 --- a/app/controllers/daily_logs_controller.rb +++ b/app/controllers/daily_logs_controller.rb @@ -1,7 +1,6 @@ class DailyLogsController < ApplicationController include DailyLogsHelper skip_before_action :verify_authenticity_token - #TODO change the flow, so that you even need to authenticate_user on index action before_action :authenticate_user, :except=>[:index] def new From 9c4c674b8b7226808dea3357c204bb6274497cc1 Mon Sep 17 00:00:00 2001 From: Ravi Bhim Date: Sat, 1 Jul 2017 10:13:20 -0700 Subject: [PATCH 17/17] ravi feedback --- app/models/daily_log.rb | 17 ++++++++++++++++- app/models/user.rb | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/models/daily_log.rb b/app/models/daily_log.rb index fc5657e..c06bc75 100644 --- a/app/models/daily_log.rb +++ b/app/models/daily_log.rb @@ -4,6 +4,8 @@ class DailyLog < ApplicationRecord validates :user_id, presence: true include ApplicationHelper + # DailyLog.create_user_summary - making sense + def self.create_user_summary(params,user_id, project_id, log_date) query_hash = {} query_hash = DailyLog.build_query_hash(params,user_id) if params.present? @@ -21,6 +23,9 @@ def self.create_user_summary(params,user_id, project_id, log_date) daily_log_summary end + # @daily_log.create_summary_record_hash + # + # TODO: Rename to create_summary_record_hash def create_summary_record(project_id) summary_record=Hash.new summary_record[:name] = self.user.name @@ -32,8 +37,16 @@ def create_summary_record(project_id) summary_record[:clients] = self.clients_project_log_record(project_id) summary_record[:logdate] = self.log_date.strftime('%v') summary_record + +=begin + summary_record = { + :name => self.user.name, + :user_id => ... + } +=end end + # @daily_log.clients_project_log_record_hash def clients_project_log_record(project_id) clients = Project.where.not(client_name: Project::LEARNING).distinct.pluck('client_name') project_logs = Hash.new() @@ -52,6 +65,7 @@ def clients_project_log_record(project_id) project_logs end + # TODO: Move transaction to this method and dont use it in the controller. def update_daily_log_and_log_entries(log_entry_records,learning,takeaway,new_log_entries) log_entry_records.each do |project_id,log_entries| log_entries.each do |log_entry_id,log_text| @@ -80,7 +94,8 @@ def self.build_query_hash(params, current_user_id) end query_hash end + def self.check_log_entry_exists(user_id,log_date) - flag = DailyLog.where(user_id: user_id, log_date:log_date).count==0 ? true : false + DailyLog.where(user_id: user_id, log_date:log_date).count==0 ? true : false end end diff --git a/app/models/user.rb b/app/models/user.rb index 309158f..a9b0c6d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,7 @@ class User < ActiveRecord::Base has_and_belongs_to_many :projects has_many :daily_logs + # TODO: write in the same line validates :name, presence: true validates :email, presence: true