会社個別 > 提出物一覧を vue.js 化した #4549

clio209 · 2022-04-02T13:23:48Z

issue 会社個別 > 提出物一覧の vue.js 化 #4311

概要

会社個別ページ>提出物一覧をvue.js化しました。以下のPRを参考にしています。

変更前

変更後

確認方法

ブランチfeature/convert-companies-products-to-vuejsl をローカルに取り込み、会社個別 > 提出物一覧画面でvue.js化されていることを確認する

clio209 · 2022-04-06T12:28:17Z

@garammasala29 さん

ユーザー個別ページ > 質問一覧の vue.js化 #4343
を参考にさせていただき、非常に勉強になりました。是非レビューをお願いできればと思います。お忙しいところ恐縮ですが、何卒よろしくお願いいたします🙇‍♂️

garammasala29

@clio209 さん
お疲れ様です。レビュー依頼ありがとうございました！遅くなってしまい申し訳ありません💦
提出物に関してあまり触れてこなかったので、とても勉強になりました！
コメントしましたので、ご確認いただけたらと思います🙏

garammasala29 · 2022-04-08T14:31:54Z

app/controllers/api/products_controller.rb


  def index
    @products = Product
                .list
                .order_for_list
                .page(params[:page])
+    @products = @products.joins(:user).where(users: { company_id: params[:company_id] }) if params[:company_id]


なるほど、joinsメソッドでテーブルを結合させて絞り込んでいるんですね👍

garammasala29 · 2022-04-08T15:05:42Z

app/views/companies/products/index.html.slim

-      .thread-list.a-card
-        = render partial: 'products/product', collection: @products, as: :product
-    = paginate @products
+#js-company-products(company-id="#{@company.id}" data-title="#{title}" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")


Suggested change

#js-company-products(company-id="#{@company.id}" data-title="#{title}" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")

#js-company-products(company-id="#{@company.id}" data-title="#{title}" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")

page-bodyを消すか、一段下げるかですかね🤔

garammasala29 · 2022-04-08T15:40:53Z

app/javascript/company-products.vue

+<script>
+import Pager from 'pager.vue'
+import Product from 'product.vue'
+export default {


Suggested change

export default {

export default {

公式ドキュメントなど1行空いていることが多い気がしますね

garammasala29 · 2022-04-08T15:49:44Z

app/javascript/company-products.js

+          props: {
+            title: title,
+            companyID: companyID,
+            isMentor: isMentor === 'true',


文字列で渡すことでwarningを回避しているんですね！

garammasala29 · 2022-04-09T09:05:47Z

app/javascript/company-products.vue

+  nav.pagination(v-if='totalPages > 1')
+    pager(v-bind='pagerProps')


Suggested change

nav.pagination(v-if='totalPages > 1')

pager(v-bind='pagerProps')

nav.pagination(v-if='totalPages > 1')

pager(v-bind='pagerProps')

こちらもcontainerの中のような気がします。

garammasala29 · 2022-04-09T09:44:52Z

app/javascript/company-products.vue

+        (this.params.tag ? `tags/${this.params.tag}` : '') +
+        `?page=${this.currentPage}` +
+        (this.params.target ? `&target=${this.params.target}` : '') +
+        (this.params.watch ? `&watch=${this.params.watch}` : '') +


tag、target、watchなどユーザーに関連するものですかね🤔

コメントありがとうございます。削除いたしました。

garammasala29 · 2022-04-09T09:46:49Z

app/javascript/company-products.vue

+    targetName() {
+      return this.currentTag || this.currentTarget
+    },


今回使用されていないのでは…と思います！(多分company-userから流用してきた流れかと😄)
こちら以外にもこのvueファイルで不必要なメソッドなどがありそうなので、一度見返していただけたらと思います🙏

有難うございます！！不必要な箇所を作事いたしました🙇‍♂️

garammasala29 · 2022-04-09T10:33:12Z

app/javascript/company-products.vue

+
+<script>
+import Pager from 'pager.vue'
+import Product from 'product.vue'


Productをインポートすることで提出者orメンターが出てしまうので、何かしら対応が必要になってくるのかな〜と思いました💦

コメント有難うございます！komagataさんに確認いたします。
@komagata さん
こちら、この表記（提出者、メンター）が表示されても問題ないでしょうか？元来誰がコメントしているのかまでアイコンで見えることから、この表記が出ても論理的に問題ないのかなとは思います。またはこの表記を消した方が良いでしょうか？

@clio209 こちらそのままで問題ございません〜

@garammasala29 さん
レビュー有難うございます！修正しましたので、お時間ある時にご確認いただけますと幸いです。

garammasala29

@clio209 さん
お疲れ様です。ご修正ありがとうございました😄
少しコメントしていますが、LGTMとさせていただきます👍

garammasala29 · 2022-04-14T06:58:17Z

app/views/products/index.html.slim

@@ -9,4 +9,4 @@ header.page-header
 - if current_user.admin? || current_user.mentor?
  = render 'products/tabs'

-#js-products(data-title="#{title}" data-selected-tab="all" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")
+  #js-products(data-title="#{title}" data-selected-tab="all" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")


今回のissueに関連はないかなと思うので、差分が出ていて気になりました！

すいません、毎回コンフリクトが起こり、色々弄って先祖返りしたりしたためかと思われます。。恥💦💦

garammasala29 · 2022-04-14T06:58:56Z

app/javascript/company-products.vue

+      if (this.params.target) {
+        return (
+          location.pathname +
+          `?target=${this.params.target}` +


ここにもまだtargetが残っていそうですね🤔

ご確認本当にありがとうございます！！

clio209 · 2022-04-14T09:09:10Z

@komagata
お手隙の際にご確認頂けますと幸いです🙇‍♂️

komagata

conflictの修正をお願いします〜

不必要なコードの削除

clio209 · 2022-04-18T08:47:49Z

@komagata
conflictの修正を行いました。お手隙の際にご確認いただけますと幸いです。

komagata

確認させて頂きました。OKです〜🙆‍♂️

clio209 self-assigned this Apr 2, 2022

clio209 force-pushed the feature/convert-companies-products-to-vuejs branch from 593cf9e to dc44442 Compare April 6, 2022 06:02

clio209 requested a review from garammasala29 April 6, 2022 12:28

clio209 marked this pull request as ready for review April 7, 2022 22:06

garammasala29 requested changes Apr 9, 2022

View reviewed changes

clio209 force-pushed the feature/convert-companies-products-to-vuejs branch 2 times, most recently from fb36639 to a735df6 Compare April 14, 2022 04:04

garammasala29 approved these changes Apr 14, 2022

View reviewed changes

clio209 requested a review from komagata April 14, 2022 09:08

komagata reviewed Apr 17, 2022

View reviewed changes

clio209 force-pushed the feature/convert-companies-products-to-vuejs branch from 7823d1c to 5473768 Compare April 17, 2022 08:15

clio209 added 13 commits April 18, 2022 14:52

companies-products.vueを作成

4ec09cc

コンフリクトの解消

a39ff55

apiコントローラー　staff限定ログインを非限定に変更

cc0c98b

コンフリクトの解消

dc00bf7

コンフリクトの解消

7c05f27

rubocop修正

8c9bdc2

Prettier 修正

f88bf37

インデント変更

01c9b5a

不必要なコードの削除

インデント、不必要なメソッドの一部を削除

8a3fb70

不必要なメソッドの一部を削除

5646ab6

prettier 修正

f985538

コンフリクトの解消

e2b18f8

rubocop 修正

df60ae1

clio209 force-pushed the feature/convert-companies-products-to-vuejs branch from e2a76a6 to df60ae1 Compare April 18, 2022 06:32

@Company に存在するときのみの条件を追加

4db6652

komagata approved these changes Apr 20, 2022

View reviewed changes

komagata merged commit ac586a8 into main Apr 20, 2022

komagata deleted the feature/convert-companies-products-to-vuejs branch April 20, 2022 04:45

github-actions bot mentioned this pull request Apr 20, 2022

Release 2022-04-21 11:15:40 #4627

Merged

20 tasks

saeyama mentioned this pull request Apr 29, 2022

会社個別 > 日報一覧を vue.js 化した #4729

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

会社個別 > 提出物一覧を vue.js 化した #4549

会社個別 > 提出物一覧を vue.js 化した #4549

clio209 commented Apr 2, 2022 •

edited

Loading

clio209 commented Apr 6, 2022

garammasala29 left a comment

garammasala29 Apr 8, 2022

garammasala29 Apr 8, 2022

garammasala29 Apr 8, 2022

garammasala29 Apr 8, 2022

garammasala29 Apr 9, 2022

garammasala29 Apr 9, 2022

clio209 Apr 11, 2022

garammasala29 Apr 9, 2022

clio209 Apr 12, 2022

garammasala29 Apr 9, 2022

clio209 Apr 12, 2022

komagata Apr 13, 2022

clio209 Apr 14, 2022

garammasala29 left a comment

garammasala29 Apr 14, 2022

clio209 Apr 14, 2022

garammasala29 Apr 14, 2022

clio209 Apr 14, 2022

clio209 commented Apr 14, 2022

komagata left a comment

clio209 commented Apr 18, 2022

komagata left a comment

	#js-company-products(company-id="#{@company.id}" data-title="#{title}" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")
	#js-company-products(company-id="#{@company.id}" data-title="#{title}" data-mentor-login="#{mentor_login?}" data-current-user-id="#{current_user.id}")

		nav.pagination(v-if='totalPages > 1')
		pager(v-bind='pagerProps')

会社個別 > 提出物一覧を vue.js 化した #4549

会社個別 > 提出物一覧を vue.js 化した #4549

Conversation

clio209 commented Apr 2, 2022 • edited Loading

概要

変更前

変更後

確認方法

clio209 commented Apr 6, 2022

garammasala29 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garammasala29 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clio209 commented Apr 14, 2022

komagata left a comment

Choose a reason for hiding this comment

clio209 commented Apr 18, 2022

komagata left a comment

Choose a reason for hiding this comment

clio209 commented Apr 2, 2022 •

edited

Loading