Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ env:
ELASTICSEARCH6_ARCHIVE: elasticsearch-6.3.1.tar.gz
OSF_DB_PORT: 5432
OSF_DB_PASSWORD: postgres
GITHUB_ACTIONS: true

jobs:
build-cache:
Expand Down Expand Up @@ -167,3 +168,36 @@ jobs:
- name: Upload report
if: (success() || failure()) # run this step even if previous step failed
uses: ./.github/actions/gen-report

mailhog:
runs-on: ubuntu-22.04
permissions:
checks: write
needs: build-cache
services:
postgres:
image: postgres

env:
POSTGRES_PASSWORD: ${{ env.OSF_DB_PASSWORD }}
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
# Maps tcp port 5432 on service container to the host
- 5432:5432
mailhog:
image: mailhog/mailhog
ports:
- 1025:1025
- 8025:8025
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/start-build
- name: Run tests
run: poetry run python3 -m invoke test-ci-mailhog -n 1 --junit
- name: Upload report
if: (github.event_name != 'pull_request') && (success() || failure()) # run this step even if previous step failed
uses: ./.github/actions/gen-report
6 changes: 2 additions & 4 deletions README-docker-compose.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,8 @@
docker compose run --rm web python3 -m scripts.parse_citation_styles
```
- Start ember_osf_web
- Needed for quickfiles feature:
```bash
docker compose up -d ember_osf_web
```
- Needed for ember app:
- `docker-compose up -d ember_osf_web`
Comment on lines -274 to +275
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • keep the format consistent (i.e. use ``` instead of `)
  • docker compose

- OPTIONAL: Register OAuth Scopes
- Needed for things such as the ember-osf dummy app
```bash
Expand Down
16 changes: 2 additions & 14 deletions addons/base/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

from addons.base import exceptions as addon_errors
from addons.base.models import BaseStorageAddon
from addons.osfstorage.models import OsfStorageFile
from addons.osfstorage.models import OsfStorageFileNode
from addons.osfstorage.utils import enqueue_update_analytics

Expand All @@ -34,7 +33,6 @@
from framework.exceptions import HTTPError
from framework.flask import redirect
from framework.sentry import log_exception
from framework.routing import proxy_url
from framework.transactions.handlers import no_auto_transaction
from website import mails
from website import settings
Expand Down Expand Up @@ -483,7 +481,7 @@ def _construct_payload(auth, resource, credentials, waterbutler_settings):

@must_be_signed
@no_auto_transaction
@must_be_valid_project(quickfiles_valid=True, preprints_valid=True)
@must_be_valid_project(preprints_valid=True)
def create_waterbutler_log(payload, **kwargs):
with transaction.atomic():
try:
Expand Down Expand Up @@ -603,7 +601,7 @@ def create_waterbutler_log(payload, **kwargs):
metadata = payload.get('metadata') or payload.get('destination')

target_node = AbstractNode.load(metadata.get('nid'))
if target_node and not target_node.is_quickfiles and payload['action'] != 'download_file':
if target_node and payload['action'] != 'download_file':
update_storage_usage_with_size(payload)

with transaction.atomic():
Expand Down Expand Up @@ -1036,16 +1034,6 @@ def persistent_file_download(auth, **kwargs):
)


def addon_view_or_download_quickfile(**kwargs):
fid = kwargs.get('fid', 'NOT_AN_FID')
file_ = OsfStorageFile.load(fid)
if not file_:
raise HTTPError(http_status.HTTP_404_NOT_FOUND, data={
'message_short': 'File Not Found',
'message_long': 'The requested file could not be found.'
})
return proxy_url(f'/project/{file_.target._id}/files/osfstorage/{fid}/')

def addon_view_file(auth, node, file_node, version):
# TODO: resolve circular import issue
from addons.wiki import settings as wiki_settings
Expand Down
47 changes: 13 additions & 34 deletions addons/boa/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ def setUp(self):
self.output_file_name = 'fake_boa_script_results.txt'
self.job_id = '1a2b3c4d5e6f7g8'

from conftest import start_mock_send_grid
self.mock_send_grid = start_mock_send_grid(self)

def tearDown(self):
super().tearDown()

Expand All @@ -52,9 +55,10 @@ def test_boa_error_code(self):
assert BoaErrorCode.FILE_TOO_LARGE_ERROR == 6
assert BoaErrorCode.JOB_TIME_OUT_ERROR == 7

@mock.patch('website.mails.settings.USE_EMAIL', True)
@mock.patch('website.mails.settings.USE_CELERY', False)
def test_handle_boa_error(self):
with mock.patch('addons.boa.tasks.send_mail', return_value=None) as mock_send_mail, \
mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message, \
with mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message, \
mock.patch('addons.boa.tasks.logger.error', return_value=None) as mock_logger_error:
return_value = handle_boa_error(
self.error_message,
Expand All @@ -68,24 +72,7 @@ def test_handle_boa_error(self):
output_file_name=self.output_file_name,
job_id=self.job_id
)
mock_send_mail.assert_called_with(
to_addr=self.user_username,
mail=ADDONS_BOA_JOB_FAILURE,
fullname=self.user_fullname,
code=BoaErrorCode.UNKNOWN,
message=self.error_message,
query_file_name=self.query_file_name,
file_size=self.file_size,
max_file_size=boa_settings.MAX_SUBMISSION_SIZE,
query_file_full_path=self.file_full_path,
output_file_name=self.output_file_name,
job_id=self.job_id,
max_job_wait_hours=self.max_job_wait_hours,
project_url=self.project_url,
boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
)
self.mock_send_grid.assert_called()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A bit lazy just to assert this but asserting every single thing in a template is unmaintainable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm ... why is it unmaintainable? Did the test fail and we can't figure out how to fix it? The reason why we assert the details for send_mail is because we want to make sure we send the email with correct information.

mock_sentry_log_message.assert_called_with(self.error_message, skip_session=True)
mock_logger_error.assert_called_with(self.error_message)
assert return_value == BoaErrorCode.UNKNOWN
Expand Down Expand Up @@ -167,9 +154,14 @@ def setUp(self):
boa_settings.REFRESH_JOB_INTERVAL = DEFAULT_REFRESH_JOB_INTERVAL
boa_settings.MAX_JOB_WAITING_TIME = DEFAULT_MAX_JOB_WAITING_TIME

from conftest import start_mock_send_grid
self.mock_send_grid = start_mock_send_grid(self)

def tearDown(self):
super().tearDown()

@mock.patch('website.mails.settings.USE_EMAIL', True)
@mock.patch('website.mails.settings.USE_CELERY', False)
async def test_submit_success(self):
with mock.patch('osf.models.user.OSFUser.objects.get', return_value=self.user), \
mock.patch('osf.models.user.OSFUser.get_or_create_cookie', return_value=self.user_cookie), \
Expand All @@ -179,7 +171,6 @@ async def test_submit_success(self):
mock.patch('boaapi.boa_client.BoaClient.query', return_value=self.mock_job), \
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None) as mock_async_sleep, \
mock.patch('addons.boa.tasks.send_mail', return_value=None) as mock_send_mail, \
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
return_value = await submit_to_boa_async(
self.host,
Expand All @@ -199,19 +190,7 @@ async def test_submit_success(self):
assert self.mock_job.refresh.call_count == 4
assert mock_async_sleep.call_count == 4
mock_close.assert_called()
mock_send_mail.assert_called_with(
to_addr=self.user.username,
mail=ADDONS_BOA_JOB_COMPLETE,
fullname=self.user.fullname,
query_file_name=self.query_file_name,
query_file_full_path=self.file_full_path,
output_file_name=self.output_file_name,
job_id=self.mock_job.id,
project_url=self.project_url,
boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
)
self.mock_send_grid.assert_called()
mock_handle_boa_error.assert_not_called()

async def test_download_error(self):
Expand Down
2 changes: 1 addition & 1 deletion addons/osfstorage/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from framework.auth import cas

from osf import features
from osf.models import Tag, QuickFilesNode
from osf.models import Tag
from osf.models import files as models
from addons.osfstorage.apps import osf_storage_root
from addons.osfstorage import utils
Expand Down
3 changes: 0 additions & 3 deletions addons/osfstorage/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,6 @@ def osfstorage_create_child(file_node, payload, **kwargs):
if not (name or user) or '/' in name:
raise HTTPError(http_status.HTTP_400_BAD_REQUEST)

if getattr(file_node.target, 'is_quickfiles', False) and is_folder:
raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={'message_long': 'You may not create a folder for QuickFiles'})

try:
# Create a save point so that we can rollback and unlock
# the parent record
Expand Down
1 change: 0 additions & 1 deletion admin/base/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
re_path(r'^maintenance/', include('admin.maintenance.urls', namespace='maintenance')),
re_path(r'^meetings/', include('admin.meetings.urls', namespace='meetings')),
re_path(r'^metrics/', include('admin.metrics.urls', namespace='metrics')),
re_path(r'^osf_groups/', include('admin.osf_groups.urls', namespace='osf_groups')),
re_path(r'^management/', include('admin.management.urls', namespace='management')),
re_path(r'^internet_archive/', include('admin.internet_archive.urls', namespace='internet_archive')),
re_path(r'^schema_responses/', include('admin.schema_responses.urls', namespace='schema_responses')),
Expand Down
2 changes: 1 addition & 1 deletion admin/common_auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class UserRegistrationForm(forms.Form):

# TODO: Moving to guardian, find a better way to distinguish "admin-like" groups from object permission groups
group_perms = forms.ModelMultipleChoiceField(
queryset=Group.objects.exclude(Q(name__startswith='collections_') | Q(name__startswith='reviews_') | Q(name__startswith='preprint_') | Q(name__startswith='node_') | Q(name__startswith='osfgroup_') | Q(name__startswith='draft_registration_')),
queryset=Group.objects.exclude(Q(name__startswith='collections_') | Q(name__startswith='reviews_') | Q(name__startswith='preprint_') | Q(name__startswith='node_') | Q(name__startswith='draft_registration_')),
required=False,
widget=forms.CheckboxSelectMultiple
)
Expand Down
5 changes: 0 additions & 5 deletions admin/nodes/templatetags/node_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ def reverse_user(user):
return reverse('users:user', kwargs={'guid': user._id})


@register.filter
def reverse_osf_group(value):
return reverse('osf_groups:osf_group', kwargs={'id': value._id})


@register.filter
def reverse_registration_provider(value):
return reverse('registration_providers:detail', kwargs={'registration_provider_id': value.provider.id})
Expand Down
Empty file removed admin/osf_groups/__init__.py
Empty file.
6 changes: 0 additions & 6 deletions admin/osf_groups/forms.py

This file was deleted.

10 changes: 0 additions & 10 deletions admin/osf_groups/urls.py

This file was deleted.

77 changes: 0 additions & 77 deletions admin/osf_groups/views.py

This file was deleted.

14 changes: 0 additions & 14 deletions admin/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,6 @@
</div>
{% endif %}
{% endif %}
{% if perms.osf.view_conference %}
<li><a href="{% url 'meetings:list' %}"><i class='fa fa-link'></i> <span>Meetings</span></a></li>
{% endif %}
{% if perms.osf.view_metrics %}
<li><a href="{% url 'metrics:metrics' %}"><i class='fa fa-link'></i> <span>Metrics</span></a></li>
{% endif %}
Expand All @@ -300,17 +297,6 @@
{% if perms.osf.view_management%}
<li><a href="{% url 'management:commands' %}"><i class='fa fa-link'></i> <span>Management Commands</span></a></li>
{% endif %}
{% if perms.osf.view_osf_groups %}
<li><a role="button" data-toggle="collapse" href="#collapseOSFGroups">
<i class='fa fa-caret-down'></i> OSF Groups
</a></li>
<div class="collapse" id="collapseOSFGroups">
<ul class="sidebar-menu sidebar-menu-inner">
<li><a href="{% url 'osf_groups:search' %}"><i class='fa fa-link'></i><span> Search</span> </a></li>
<li><a href="{% url 'osf_groups:osf_groups_list' %}"><i class='fa fa-link'></i><span> List</span> </a></li>
</ul>
</div>
{% endif %}
{% if perms.osf.view_scheduledbanner %}
<li><a role="button" data-toggle="collapse" href="#collapseBanners">
<i class='fa fa-caret-down'></i> Banners
Expand Down
Loading
Loading