Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Feature/sessions #55

Merged
merged 7 commits into from
Jul 24, 2024
Merged

Feature/sessions #55

merged 7 commits into from
Jul 24, 2024

Conversation

6intez
Copy link
Collaborator

@6intez 6intez commented Jul 23, 2024

No description provided.

Copy link
Collaborator

@mikeGEINE mikeGEINE left a comment

Choose a reason for hiding this comment

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

Кажется, что комментариев много, но в основном это либо что-то перенести в другое место, либо убрать. Думаю, можно быстро пофиксить)

app/api/vk.rb Outdated Show resolved Hide resolved
app/api/vk.rb Outdated
def self.get_friends(user)
resp = try_request(:get,
'/method/friends.get',
{ access_token:user.access_token,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Этот запрос точно так работает? Имхо, лучше токен не передавать в параметрах, мы его передаём в хэдерах

app/api/vk.rb Outdated
Comment on lines 61 to 73
response_info=try_request(:get,
'/method/users.get',
{access_token: user.access_token,
v: '5.199',
user_ids: friends_id_list_str,
fields: 'photo_200'},
client:)
response_info_hash=response_info.body['response']['items']
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Это лишний запрос. friends.get возвращает список друзей с их именами + в fields можно указать дополнительные поля, среди которых есть фотография. Нам хватит photo_100

app/api/vk.rb Outdated
{ access_token: user.access_token,
user_ids: user.user_id,
v: '5.199',
fields: 'first_name last_name' },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нам не нужно здесь запросить ещё аватар пользователя?

app/api/vk.rb Outdated
Comment on lines 116 to 143
post_data.each do |post_data|
like_count = post_data['likes']['count']
comment_count = post_data['comments']['count']

if(like_count==0)
post_likers=[]
else
post_likers = get_likers(post_data['id'], like_count.to_i, user)
end
if(comment_count==0)
post_commentators=[]
else
post_commentators = get_commentators(post_data['id'], comment_count.to_i, user)
end
post = {
post_id: post_data['id'],
date: post_data['date'],
image_url: get_image_from_post(post_data['attachments']),
count_likes: post_data['likes']['count'],
count_comments: post_data['comments']['count'],
likers: post_likers,
commentators: post_commentators
}
posts << post
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Кажется, если вместо each здесь вызвать map, то не нужно будет делать `posts << post', а просто остановится на том, что собирается хэш с данными. И вся функция закончится на этом

sign_out @current_user
redirect_to root_path
else
puts('no user founded')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
puts('no user founded')
puts('no user found')

Или

Suggested change
puts('no user founded')
puts('user not found')


def user_sign_out
if user_signed_in?
sign_out @current_user
Copy link
Collaborator

Choose a reason for hiding this comment

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

Точно @current_user? Devise имеет метод current_user, думаю, он лучше подойдёт для задачи

@@ -0,0 +1,5 @@
class AddStatetoUsers < ActiveRecord::Migration[7.1]
def change
add_column :users, :user_state, :string
Copy link
Collaborator

Choose a reason for hiding this comment

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

За что отвечает state?

end
end

def user_sign_out
Copy link
Collaborator

Choose a reason for hiding this comment

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

У Devise есть собственный action для разлогина пользователя, см. вывод команды 'rails routes'

@@ -1,4 +1,11 @@
# frozen_string_literal: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если Devise всё же работает с пользователем, то эти методы излишни

@mikeGEINE
Copy link
Collaborator

@vzalygin как думаешь, почему CI не нашел рубокопа?

@vzalygin
Copy link
Collaborator

@mikeGEINE, не знаю, беды с зависимостями. Но ветка старая, надо попробовать сковшнуть все коммиты и ребейзнуть их на свежий dev, вероятно поможет

@vzalygin
Copy link
Collaborator

git stash -u && git fetch -a && git rebase -i origin/dev && git stash pop

@vzalygin
Copy link
Collaborator

и на моменте git rebase -i origin/dev надо для всех коммитов кроме первого выбрать сквош

@mikeGEINE
Copy link
Collaborator

@mikeGEINE, не знаю, беды с зависимостями. Но ветка старая, надо попробовать сковшнуть все коммиты и ребейзнуть их на свежий dev, вероятно поможет

Смотри, там в гемфайле прописано gem 'rubocop', '~> 1.65', require: false . Мб из-за этого в некоторых случаях у нас гем не ставится?

working sessions/without refreshing token

working sessions/without refreshing token;2D

working sessions/without refreshing token;2D

refresh tokens

change refresh tokens

Some suggestions

changes module:vk

changes module:vk#2

changes module:vk#3//posts

some fixes in refreshing

some changes

rubocop for vk.rb

rubocop sessions

changes
@mikeGEINE mikeGEINE merged commit 8c649df into dev Jul 24, 2024
1 check failed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants