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
Merged

Conversation

jorg-vr
Copy link
Contributor

@jorg-vr jorg-vr commented Feb 3, 2023

This pull request fixes multiple paper cuts related to repositories.

  • The placeholder for the clone url on the repositories/new page mentions github.ugent.be. While correct, github.com would be more logical. From Paper cuts #4240

  • When adding a repo, add a better error message when permissions are missing. From Paper cuts #4240
    New message:
    image

  • The learning activities table (at least the one on the repo page) has no empty state. From Paper cuts #4240
    New empty state:
    image

  • When adding a repository, the existing exercises were not added automatically. From Paper cuts #4240
    This issue was only present in staging and production, where the git worker queue allowed the tasks to be done simultaneously/out of order. This should now be forced to consecutive execution.

Part of #4362 .

@jorg-vr jorg-vr added the enhancement A change that isn't substantial enough to be called a feature label Feb 3, 2023
@jorg-vr jorg-vr self-assigned this Feb 3, 2023
@jorg-vr jorg-vr added the deploy naos Request a deployment on naos label Feb 6, 2023
@jorg-vr jorg-vr temporarily deployed to naos February 6, 2023 09:50 — with GitHub Actions Inactive
@github-actions github-actions bot removed the deploy naos Request a deployment on naos label Feb 6, 2023
@jorg-vr jorg-vr changed the title Fix/repo paper cuts Fix multiple repository related paper cuts Feb 7, 2023
@jorg-vr jorg-vr added bug Something isn't working and removed enhancement A change that isn't substantial enough to be called a feature labels Feb 7, 2023
@jorg-vr jorg-vr marked this pull request as ready for review February 7, 2023 10:04
@jorg-vr jorg-vr requested a review from a team as a code owner February 7, 2023 10:04
@jorg-vr jorg-vr requested review from bmesuere and niknetniko and removed request for a team February 7, 2023 10:04
config/locales/models/en.yml Outdated Show resolved Hide resolved
config/locales/models/nl.yml Outdated Show resolved Hide resolved
Co-authored-by: Niko Strijbol <strijbol.niko@gmail.com>
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

I don't like the email solution either. Can't we just send an email to the address associated with the repo admin in this case? Since the repo has just been added by that user, using that address seems like a good fallback.

config/locales/models/en.yml Outdated Show resolved Hide resolved
@jorg-vr
Copy link
Contributor Author

jorg-vr commented Feb 8, 2023

the repo admin is only added after the repo is saved.
Because cloning the repo is a delayed task, and it also takes some time to perform, this admin will almost always be added before the start of process activities.
However, this order of execution is not at all guaranteed and could result into bugs in some cases. (Most notably it causes this bug in our test environment where deleyad_jobs are executed in order)

This was my first solution which I reverted because of a failing test. Do you want to continue with this approach? (In which case I would just remove the test because we know it will fail)

@bmesuere
Copy link
Member

bmesuere commented Feb 8, 2023

In that case, maybe add dodona@ugent.be as ultimate fallback?

@jorg-vr jorg-vr requested a review from bmesuere February 8, 2023 15:38
@jorg-vr jorg-vr merged commit ffbf132 into develop Feb 8, 2023
@jorg-vr jorg-vr deleted the fix/repo-paper-cuts branch February 8, 2023 15:52
@bmesuere bmesuere added enhancement A change that isn't substantial enough to be called a feature and removed bug Something isn't working labels Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A change that isn't substantial enough to be called a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants