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: handle case sensitivity for "Settings" sheet name with explicit error TASK-1353 #5417

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

noliveleger
Copy link
Contributor

@noliveleger noliveleger commented Jan 13, 2025

πŸ“£ Summary

Improved error handling for case-sensitive "Settings" sheet names.

πŸ“– Description

This update addresses an issue where a sheet named "Settings" with uppercase or mixed case letters causes unexpected behavior. An explicit error message is now raised to alert users of the case sensitivity, ensuring they can resolve the issue easily.

πŸ‘€ Preview steps

  1. Upload this project
  2. Try to deploy the project.
  3. πŸ”΄ [on main] notice the modal returns a 500
  4. 🟒 [on PR] notice the modal exposes a human-readable error

@noliveleger noliveleger changed the title Handle Settings sheet name fix: Handle case sensitivity for "Settings" sheet name with explicit error Jan 13, 2025
@noliveleger noliveleger self-assigned this Jan 13, 2025
@noliveleger noliveleger changed the title fix: Handle case sensitivity for "Settings" sheet name with explicit error fix: handle case sensitivity for "Settings" sheet name with explicit error Jan 13, 2025
@noliveleger noliveleger requested a review from Guitlle January 13, 2025 20:43
Copy link
Contributor

@Guitlle Guitlle left a comment

Choose a reason for hiding this comment

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

good

@noliveleger noliveleger changed the title fix: handle case sensitivity for "Settings" sheet name with explicit error fix: handle case sensitivity for "Settings" sheet name with explicit error TASK-1353 Jan 13, 2025
@noliveleger noliveleger merged commit 9189ac9 into release/2.024.36 Jan 14, 2025
6 checks passed
@noliveleger noliveleger deleted the task-1353-xlsxwriter-error-handling branch January 14, 2025 14:40
noliveleger added a commit that referenced this pull request Jan 14, 2025
)

### πŸ“£ Summary
Enhanced error handling to catch more validation errors in XLSForm
during deployment.


### πŸ“– Description
Validation error handling for XLSForm deployment has been enhanced to
catch a wider range of issues. This prevents the display of a generic
500 error in the deployment modal and instead returns the explicit error
message.

### Notes
Supersedes #5417, #5411 and #5403
magicznyleszek added a commit that referenced this pull request Jan 17, 2025
commit 11ee676
Author: Rebecca Graber <becca.graber@kobotoolbox.org>
Date:   Thu Jan 16 08:47:09 2025 -0500

    feat(projectHistoryLogs): log new submissions (#5416)

    ### πŸ“£ Summary
    Create logs when new submissions are added to projects.

    ### πŸ‘· Description for instance maintainers
    Allow null user_uids in AuditLogs so we can log anonymous submissions.

    ### πŸ’­ Notes
    We previously had no need for null users in audit logs because the
    actions we logged were all restricted to authenticated users, but since
    we allow anonymous submissions, we needed a way to log those.

    ### πŸ‘€ Preview steps

    Feature/no-change template:
    1. ℹ️ have an account and a project. Make sure the account username is
    not `admin` (see [this notion
    task](https://www.notion.so/kobotoolbox/Anonymous-submissions-dont-work-if-user-named-admin-owns-asset-1767e515f65480608dfcee76ba9b3710?pvs=4))
    2. Deploy the project
    3. Add a submission to the project
    4. Go to `api/v2/asset/<asset-uid>/history`
    5. 🟒 There should be a new project history log with
    `action='add-submission'` and all the usual metadata, plus
    ```
    "submission": {
        "submitted_by": "user1"
    }
    ```
    6. Enable submissions without username/email to the project
    7. To make sure you're submitting anonymously, copy and paste the enketo
    link into a new private tab and add a new submission
    8. 🟒 Reload the endpoint. There should be a new audit log with
    `action='add-submission'`
    a. The user should be
    `http://kf.kobo.local:8080/api/v2/users/AnonymousUser/`
      b. The user_uid will be the uid of the anonymous user in the database
      c. The username should be `AnonymousUser`
    d. The metadata should contain `{"submission": {"submitted_by":
    "AnonymousUser"}` in addition to the usual

commit bbfdaf1
Author: olive-KTB <olive.godswill@kobotoolbox.org>
Date:   Thu Jan 16 02:49:14 2025 +0100

    update gitlab-ci.yml

commit bc56f8f
Author: Akuukis <Akuukis@users.noreply.github.com>
Date:   Wed Jan 15 10:50:39 2025 +0200

    refactor(frontend): Mantine Component Library PoC (#5344)

    ### πŸ’­ Notes

    Please read the PR commit-by-commit, here's a guide.

    1.
    [3105f47](3105f47)
    Setup Mantine Component Library. Please install the new dependencies,
    and add recommended VSCode extensions.
    2.
    [453f2c1](453f2c1)
    Here's a preview that Mantine in general works, and API for the example
    Button is different but generally similar.

    | our button | Mantine default button |
    |--------|--------|
    |
    ![image](https://github.com/user-attachments/assets/1f3e5736-c5eb-4cbf-a56e-a393dab561d3)
    |
    ![image](https://github.com/user-attachments/assets/c8f2a9dc-cf1a-44c3-881a-ff05da433060)
    |

    ![image](https://github.com/user-attachments/assets/e2abe407-3113-4900-9027-43186137ce13)

    3.
    [d782e38](d782e38)
    Example of custom styled component, Button. IMHO achieves pixel-perfect
    match in storybook and example above, except for line breaks, spinner
    and hover animations. Click animation matches out of box. Icon-only
    buttons are omitted because Mantine uses a different component
    `IconAction` for those.

    | Original on left / Mantine implementation on right |
    | --- |
    |
    ![image](https://github.com/user-attachments/assets/9a58b7af-3ff4-4b18-995c-6c57ff4264cb)
    |

    5.
    [eaf45e3](eaf45e3)
    Wrapped Button to add support for inbuilt Tooltip. No idea if we want to
    move forward with these two coupled, but I found it useful for
    comparison by re-implementing part of old Button behavior that's
    represented in storybook.

    | Original | Mantine implementation |
    | --- | --- |
    |
    ![image](https://github.com/user-attachments/assets/7bf2a504-902b-4801-a2dd-3e7648166316)
    |
    ![image](https://github.com/user-attachments/assets/41bc4dff-acf3-4328-ab6f-f7fcccbde20b)
    |

    ### πŸ‘€ Preview steps

    1. ℹ️ open Kobo home
    4. 🟒 [on main] notice the original "new" button
    6. 🟒 [on PR] notice the new "new" button with default mantine style,
    only slightly different

    ---------

    Co-authored-by: Leszek Pietrzak <leszek@magicznyleszek.xyz>
    Co-authored-by: Leszek <smutnyleszek@gmail.com>
    Co-authored-by: James Kiger <68701146+jamesrkiger@users.noreply.github.com>
    Co-authored-by: Paulo Amorim <pauloamorimbr@gmail.com>
    Co-authored-by: James Kiger <james.kiger@gmail.com>

commit 9598180
Author: Raj Patel <51355159+rajpatel24@users.noreply.github.com>
Date:   Wed Jan 15 13:29:03 2025 +0530

    fix!: reject duplicate submissions (#5047)

    ## Summary
    Implemented logic to detect and reject duplicate submissions.

    ## Description

    We have identified a race condition in the submission processing that
    causes duplicate submissions with identical UUIDs and XML hashes. This
    issue is particularly problematic under conditions with multiple remote
    devices submitting forms simultaneously over unreliable networks.

    To address this issue, a PR has been raised with the following proposed
    changes:

    - Race Condition Resolution: A locking mechanism has been added to
    prevent the race condition when checking for existing instances and
    creating new ones. This aims to eliminate duplicate submissions.

    - UUID Enforcement: Submissions without a UUID are now explicitly
    disallowed. This ensures that every submission is uniquely identifiable
    and further mitigates the risk of duplicate entries.

    - Introduction of `root_uuid`:

    - To ensure a consistent submission UUID throughout its lifecycle and
    prevent duplicate submissions with the same UUID, a new `root_uuid`
    column has been added to the `Instance` model with a unique constraint
    (`root_uuid` per `xform`).

    - If the `<meta><rootUuid>` is present in the submission XML, it is
    stored in the `root_uuid` column.

    - If `<meta><rootUuid>` is not present, the value from
    `<meta><instanceID>` is used instead.

    - This approach guarantees that the `root_uuid` remains constant across
    the lifecycle of a submission, providing a reliable identifier for all
    instances.

    - UUID Handling Improvement: Updated the logic to strip only the `uuid:`
    prefix while preserving custom, non-UUID ID schemes (e.g.,
    domain.com:1234). This ensures compliance with the OpenRosa spec and
    prevents potential ID collisions with custom prefixes.

    - Error Handling:
    - 202 Accepted: Returns when content is identical to an existing
    submission and successfully processed.
    - 409 Conflict: Returns when a duplicate UUID is detected but with
    differing content.

    These changes should improve the robustness of the submission process
    and prevent both race conditions and invalid submissions.

    ## Notes

    - Implemented a fix to address the race condition that leads to
    duplicate submissions with the same UUID and XML hash.
    - Incorporated improvements from existing work, ensuring consistency and
    robustness in handling concurrent submissions.
    - The fix aims to prevent duplicate submissions, even under high load
    and unreliable network conditions.

    ## Related issues

    Supersedes
    [kobotoolbox/kobocat#876](kobotoolbox/kobocat#876)
    and kobotoolbox/kobocat#859

    ---------

    Co-authored-by: Olivier Leger <olivierleger@gmail.com>

commit d22b8b5
Merge: 89bd9b7 b4aa1b7
Author: John N. Milner <john@tmoj.net>
Date:   Tue Jan 14 15:20:49 2025 -0500

    Merge remote-tracking branch 'origin/release/2.024.36'

commit 89bd9b7
Author: Rebecca Graber <becca.graber@kobotoolbox.org>
Date:   Tue Jan 14 15:11:37 2025 -0500

    fix(auditLogs): correctly serialize audit logs from deleted users (#5418)

    ### πŸ“£ Summary
    Fixes a 500 error from the various audit log endpoints when there are
    actions by deleted users.

    ### πŸ“– Description
    Return empty user and username fields in the response if the user was
    deleted after the log was created. This applies to `/api/v2/audit-logs`,
    `api/v2/assets/<uid>/history`, and `api/v2/project-history-logs`.

    ### πŸ’­ Notes
    Small fix in the serializer. Also updates the ProjectHistoryLog
    serializer to inherit from the AuditLogSerializer so we don't have to
    duplicate the method fields.

    ### πŸ‘€ Preview steps

    Bug template:
    1. ℹ️ have a super user account and a project
    2. Create a new user (user1) and give them the `Edit Form` permission on
    the project.
    3. Log in as user1 and make an edit to the project.
    4. Log out user1 and log back in as the super user
    5. Delete user1. You can do this from the admin page if you delete the
    user from the User list, then from the Trash Bin.
    6. Go to:
      a. `api/v2/audit-logs`
      b. `api/v2/project-history-logs`
      c. `api/v2/assets/<uid>/history`
    7. πŸ”΄ [on main] All will return a 500 error (`AttributeError: 'NoneType'
    object has no attribute 'username'`)
    8. 🟒 [on PR] The endpoint will return the expected logs. For all user1's
    actions, the user and username fields will be empty. but the user_uid
    should still refer to the old user.

commit b4aa1b7
Author: jnm <john@tmoj.net>
Date:   Tue Jan 14 15:01:18 2025 -0500

    feat: export background-geopoint as GPS field (#5420)

    See kobotoolbox/formpack#327. This change just updates the formpack
    commit hash used by KPI

commit dddd619
Author: Olivier LΓ©ger <olivierleger@gmail.com>
Date:   Tue Jan 14 14:10:31 2025 -0500

    fix: catch additional XLSForm validation errors during deployment (#5419)

    ### πŸ“£ Summary
    Enhanced error handling to catch more validation errors in XLSForm
    during deployment.

    ### πŸ“– Description
    Validation error handling for XLSForm deployment has been enhanced to
    catch a wider range of issues. This prevents the display of a generic
    500 error in the deployment modal and instead returns the explicit error
    message.

    ### Notes
    Supersedes #5417, #5411 and #5403

commit 9189ac9
Author: Olivier LΓ©ger <olivierleger@gmail.com>
Date:   Tue Jan 14 09:40:34 2025 -0500

    fix: handle case sensitivity for "Settings" sheet name with explicit error TASK-1353 (#5417)

    ### πŸ“£ Summary
    Improved error handling for case-sensitive "Settings" sheet names.

    ### πŸ“– Description
    This update addresses an issue where a sheet named "Settings" with
    uppercase or mixed case letters causes unexpected behavior. An explicit
    error message is now raised to alert users of the case sensitivity,
    ensuring they can resolve the issue easily.

commit 8e8d6bb
Author: Rebecca Graber <becca.graber@kobotoolbox.org>
Date:   Mon Jan 13 15:02:04 2025 -0500

    test: rename admin user in fixture (#5415)

    ### πŸ’­ Notes
    Developer-facing changes only. Changes the username of the admin user to
    `adminuser` in preparation for disallowing the name `admin` as part of
    https://www.notion.so/kobotoolbox/Anonymous-submissions-dont-work-if-user-named-admin-owns-asset-1767e515f65480608dfcee76ba9b3710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants