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

MVP for agenda api #879

Merged
merged 60 commits into from
Dec 12, 2023
Merged

MVP for agenda api #879

merged 60 commits into from
Dec 12, 2023

Conversation

AlexandreJunod
Copy link
Collaborator

@AlexandreJunod AlexandreJunod commented Oct 5, 2023

Required points before merging embed-agenda in develop

  • Secure image
  • Filter by domain
  • Add agenda-embed build from @monodo
  • Restrict fields according to visible submissions, according to domain
  • Fix and refactor SITE_DOMAIN and get_absolute_url
  • Fix bad domain given
  • Return original image size in serializer
  • Make filter startAt and endAt work properly
  • Publication agenda instead of Publication calendrier
  • CANCELED Dinstinct filters in agenda, bug mutiplies X times. X being the number of forms
  • In JS, hide filter for api in fields when type is not correct
  • Make poster mandatory in fixture
  • Check that featured item comes first
  • Remove agenda app that was there for developping purposes only
  • ⚠️ WRITE TESTS

@monodo

  • Check agenda with every submission status (To be sure we defined it correctly)
  • Check if order of items in agenda is correct, or should it be changed on QS 🐛 ? featured items do NOT come first
  • Check if featured (checkbox in admin) work properly 🐛 ? featured items do NOT come first
  • Check if filters in /rest/agenda are isolated correctly with large data (preprod)
  • Check if filters in /rest/agenda/domain return only :
    • current domain (entity)
  • Check security of images /rest/image/ and /rest/image/thumbor/
  • Make a last review
  • Review documentation https://github.com/yverdon/geocity/wiki/agenda
  • Resolve all comments

@AlexandreJunod

  • Check agenda with every submission status (To be sure we defined it correctly)
  • Check if order of items in agenda is correct, or should it be changed on QS
  • Check if featured (checkbox in admin) work properly
    • Check if the "bad code" is removed for featured in serializer
  • Check if filters in /rest/agenda are isolated correctly with large data (preprod)
  • Check if filters in /rest/agenda return only :
    • current domain (entity)
  • Check security of images /rest/image/ and /rest/image/thumbor/
  • Read all the code and remove every TODO. Tag IMPROVE V2 things not required in V1
  • Make a last review
  • Resolve all comments

geocity/apps/agenda/views.py Outdated Show resolved Hide resolved
Comment on lines 899 to 900
"width": "1365",
"height": "2048",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Define how we can have width and height ? Is it the actual image width and height ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will be depend on thumbor specs that will resize depending on what the web component asks for

Copy link
Collaborator Author

@AlexandreJunod AlexandreJunod Nov 16, 2023

Choose a reason for hiding this comment

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

This is not how it's working. I've been doing some tests, and this value is required to show an image. We can still resize the image through Thumblor. Seems like to be the original file dimensions.
@monodo

geocity/apps/api/serializers.py Outdated Show resolved Hide resolved
geocity/apps/api/views.py Outdated Show resolved Hide resolved
geocity/apps/submissions/management/commands/fixturize.py Outdated Show resolved Hide resolved


# TODO: Place this view in the form or submission, so we have direct access to the image settings, public or not ? From API
def image_display(request, form_id, image_name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want once to display image in the the "geo-calendar", we'll have to duplicate this code in current api ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically no, but if it's the case, we should not put this in an app called "agenda".
We should use another name, so we can group data providers in a specific app for this usage.
The reason to put this in another app is to make it easier to find all the code that has the same purpose, and to have a complete URL that doesn't start with /submissions/

image_dir, f"permit_requests_uploads/{form_id}/{image_name}"
)

# TODO: Ajouter de la sécurité afin de savoir si l'image peut-être affichée ou non
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some help to ensure only public images are returned (not tested):

   field_value = get_object_or_404(
        models.FieldValue.objects.filter(field__input_type=Field.INPUT_TYPE_FILE,is_public_when_permitrequest_is_public=True),
        pk=property_value_id,
        field_values__field__public_info=True,
    )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

geocity/apps/api/serializers.py Outdated Show resolved Hide resolved
geocity/apps/api/serializers.py Outdated Show resolved Hide resolved
geocity/apps/api/serializers.py Show resolved Hide resolved
geocity/apps/forms/models.py Show resolved Hide resolved
_("Visible dans l'agenda"),
default=False,
help_text=_(
"""Lorsque cette case est cochée, les données de ce formulaire sont accessibles dans l'API <b>/rest/agenda/</b>"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

"""Lorsque cette case est cochée, les données de ce formulaire sont accessibles dans l'API /rest/agenda/ si l'annonce est rendue publique par le pilote"""

Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Check that all filters ensure only pulic fields for public submissions are visible

Copy link
Collaborator Author

@AlexandreJunod AlexandreJunod Nov 1, 2023

Choose a reason for hiding this comment

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

1 : Done
2: TODO Added on the right place on the code

_("Visible dans l'API light"),
default=False,
help_text=_(
"""Lorsque cette case est cochée, ce champ est affichée dans la version light de l'api (/rest/RESSOURCE).<br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ uniquement si la demande, l'annonce, est publique
affichée => affiché

Copy link
Collaborator Author

@AlexandreJunod AlexandreJunod Nov 1, 2023

Choose a reason for hiding this comment

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

"si la demande est rendue publique par le pilote" added on the help text, like the comment above

geocity/tests/factories.py Outdated Show resolved Hide resolved
@AlexandreJunod AlexandreJunod marked this pull request as ready for review November 16, 2023 14:02
Copy link
Collaborator

@monodo monodo left a comment

Choose a reason for hiding this comment

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

  • think about updating the wiki

INTERNAL_WEB_ROOT_URL = "http://web:9000"
image_url = f"{INTERNAL_WEB_ROOT_URL}/rest/image/{submission_id}/{image_name}"

# TODO: understand (adaptive-)(full-)fit-in between unsafe and size
Copy link
Collaborator

Choose a reason for hiding this comment

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

optional => v2

@@ -870,7 +877,7 @@ class Field(models.Model):
max_length=255,
blank=True,
help_text=_(
'Ex: "Yverdon-les-Bains" afin de limiter les recherches à Yverdon, <a href="https://api3.geo.admin.ch/services/sdiservices.html#search" target="_blank">Plus d\'informations</a>'
'Ex: "Yverdon-les-Bains" afin de limiter les recherches à Yverdon, <a href="https://api3.Sgeo.admin.ch/services/sdiservices.html#search" target="_blank">Plus d\'informations</a>'
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

<agenda-embed api-base-url="http://localhost:9095/rest"></agenda-embed>

<!-- Initialize the component -->
<script type="module" src="{% static 'js/agenda-embed/agenda-embed.js' %}"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove js code from static and use code that is made available throught build process in statics

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@monodo
Copy link
Collaborator

monodo commented Nov 29, 2023

image

Add to the comment: "Le pilote peut alors contrôler la publication dans l'agenda dans l'onglet traitement

@monodo
Copy link
Collaborator

monodo commented Nov 29, 2023

Looks like the ordering of items is mixed up:

image

geocity/apps/django_wfs3/pagination.py Outdated Show resolved Hide resolved
@@ -138,6 +140,7 @@
"geocity.apps.reports",
"geocity.apps.forms",
"geocity.apps.submissions",
"geocity.apps.agenda",
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove before merge

geocity/tests/api/test_agenda_api.py Outdated Show resolved Hide resolved
geocity/apps/django_wfs3/pagination.py Outdated Show resolved Hide resolved
@monodo monodo merged commit 6f86aa7 into develop Dec 12, 2023
2 checks passed
@AlexandreJunod AlexandreJunod deleted the feature/ylba-24 branch December 13, 2023 11:46
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.

2 participants