Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple repository related paper cuts #4383

Merged
merged 12 commits into from
Feb 8, 2023
1 change: 0 additions & 1 deletion app/controllers/repositories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def create
if saved
Event.create(event_type: :exercise_repository, user: current_user, message: "#{@repository.name} (id: #{@repository.id})")
RepositoryAdmin.create(user_id: current_user.id, repository_id: @repository.id)
@repository.delay(queue: 'git').process_activities_email_errors(user: current_user)
end

respond_to do |format|
Expand Down
4 changes: 2 additions & 2 deletions app/models/concerns/gitable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ def clone_incomplete?

def repo_is_accessible
cmd = ['git', 'ls-remote', remote.shellescape]
_out, error, status = Open3.capture3(*cmd)
errors.add(:remote, error) unless status.success?
_out, _error, status = Open3.capture3(*cmd)
errors.add(:remote, I18n.t('activerecord.errors.models.gitable.repository.not_accessible_markdown')) unless status.success?
end

def github_remote?
Expand Down
8 changes: 8 additions & 0 deletions app/models/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,15 @@ def activity_dirs
activity_dirs_below(full_path)
end

def clone_repo
super
process_activities_email_errors if clone_complete?
end

def process_activities_email_errors(kwargs = {})
kwargs[:user] = admins.first if kwargs.empty? && admins.any?
kwargs[:email] = Rails.application.config.dodona_email if kwargs.empty?

process_activities
rescue AggregatedConfigErrors => e
ErrorMailer.json_error(e, **kwargs).deliver
Expand Down
5 changes: 5 additions & 0 deletions app/views/activities/_activities_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@
</tbody>
</table>
</div>
<% if local_assigns[:activities].empty? %>
<p class="text-center text-muted lead table-placeholder">
<%= t "activities.index.empty" %>
</p>
<% end %>
<% if activities.try(:total_pages) %>
<center><%= page_navigation_links activities, true, "activities" %></center>
<% end %>
2 changes: 1 addition & 1 deletion app/views/judges/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<div class="col-sm-6">
<%= f.text_field :remote,
class: "form-control",
placeholder: "git@github.ugent.be/...",
placeholder: "git@github.com/...",
disabled: judge.new_record?.! %>
</div>
<span class="help-block offset-sm-3 col-sm-6">
Expand Down
4 changes: 2 additions & 2 deletions app/views/repositories/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<h4><%= t('errors.validation_errors', :count => repository.errors.count) %></h4>
<ul>
<% repository.errors.full_messages.each do |message| %>
<li><%= message %></li>
<li><%= markdown message %></li>
<% end %>
</ul>
</div>
Expand All @@ -25,7 +25,7 @@
<div class="col-sm-6">
<%= f.text_field :remote,
class: "form-control",
placeholder: "git@github.ugent.be/...",
placeholder: "git@github.com/...",
disabled: repository.new_record?.! %>
</div>
<span class="help-block offset-sm-3 col-sm-6">
Expand Down
2 changes: 1 addition & 1 deletion app/views/repositories/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
</tbody>
</table>
<% else %>
<p class="lead text-center"><%= t ".public_files_placeholder"%></p>
<p class="lead text-center text-muted "><%= t ".public_files_placeholder"%></p>
<% end %>
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions config/locales/models/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ en:
attributes:
activity:
taken: You have already read this activity
gitable:
repository:
not_accessible_markdown: The repository is not accessible to our git user. Please double check that you have given the correct permissions. Note that it may take a while before a GitHub/GitLab invitation is accepted by our team. See [the documentation](https://docs.dodona.be/en/guides/teachers/new-exercise-repo/) for more information.
models:
course: Course
activity: Activity
Expand Down
3 changes: 3 additions & 0 deletions config/locales/models/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ nl:
attributes:
activity:
taken: Je hebt deze activiteit al gelezen
gitable:
repository:
not_accessible_markdown: De repository is niet toegankelijk voor onze git-gebruiker. Controleer of je de juiste permissies hebt ingesteld. Hou er rekening mee dat het even kan duren eer een uitnodiging voor GitHub/GitLab wordt geaccepteerd door ons team. Bekijk [de documentatie](https://docs.dodona.be/nl/guides/teachers/new-exercise-repo/) voor meer informatie.
models:
course: Cursus
activity: Activiteit
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/activities/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ en:
path: "Path"
class_progress: Class progress
class_progress_visibility_disabled: "The class progress is not visible to students."
empty: "No activities found"
progress_chart_info_tried:
one: "One user started this exercise."
other: "%{count} users started this exercise."
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/activities/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ nl:
path: "Pad"
class_progress: Voortgang groep
class_progress_visibility_disabled: "De voortgang van de groep is niet zichtbaar voor studenten."
empty: "Geen activiteiten gevonden"
progress_chart_info_tried:
one: "Één gebruiker begon aan deze oefening."
other: "%{count} gebruikers begonnen aan deze oefening."
Expand Down
15 changes: 8 additions & 7 deletions test/controllers/repositories_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ def request_public_image

test_crud_actions

test 'should process activities on create' do
Repository.any_instance.expects(:process_activities)
create_request_expect
end

test 'should reprocess activities' do
Repository.any_instance.expects(:process_activities)
get reprocess_repository_path(@instance)
Expand Down Expand Up @@ -207,6 +202,14 @@ def find_echo
assert_equal 'private', find_echo.access
end

test 'should process activities on create' do
Repository.any_instance.expects(:process_activities)
user = users(:staff)
judge = create :judge, :git_stubbed
sign_in user
post repositories_path, params: { repository: { name: 'test', remote: @remote.path, judge_id: judge.id } }
end

test 'should email during repository creation' do
user = users(:staff)
judge = create :judge, :git_stubbed
Expand All @@ -215,8 +218,6 @@ def find_echo
assert_difference 'ActionMailer::Base.deliveries.size', +1 do
post repositories_path, params: { repository: { name: 'test', remote: @remote.path, judge_id: judge.id } }
end
email = ActionMailer::Base.deliveries.last
assert_equal [user.email], email.to
end

test 'github webhook with commit info should update exercises' do
Expand Down