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

Clean up unused enterprise fields #12460

Merged
merged 3 commits into from
May 22, 2024

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented May 10, 2024

What? Why?

The less code the better. When talking about new APIs, we were trying to map all existing attributes but that's wasted effort on unused attributes. We better avoid that in the future.

What should we test?

  • Test that the pickup time for an order cycle displays correctly.

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@mkllnk mkllnk self-assigned this May 10, 2024
@mkllnk mkllnk added the user facing changes Thes pull requests affect the user experience label May 10, 2024
@mkllnk mkllnk marked this pull request as ready for review May 10, 2024 05:43
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Good cleanup 🧹

Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Great work!

@filipefurtad0
Copy link
Contributor

Hey @mkllnk ,
there was a recent merge causing a conflict on this one.

This was actually shown in one place and represents a user-facing
change. But you weren't able to edit the field which means that only
very old enterprises would have had this field set and were not able to
change it anymore.

I searched au-prod and found the following values in the database:

- "Friday 31st January"
- "From 4pm, Monday 30 September"
- "From 5pm-7pm Monday"
- "Saturday 27 April 12noon"
- "January 31st/February 1st"
- "Saturday 1st February"

They seem specific to a certain order cycle and have no value as
fallback any more. Seems safe to remove.
@mkllnk
Copy link
Member Author

mkllnk commented May 15, 2024

Rebased, ready for testing again.

@filipefurtad0 filipefurtad0 self-assigned this May 22, 2024
@filipefurtad0 filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label May 22, 2024
@filipefurtad0
Copy link
Contributor

Hey @mkllnk ,

I've checked the usual places for the pick up time after staging this PR:

Shopfront:

image

On top of all three checkout steps:

image

Order confirmation screen:

image

Email confirmations (displaying one email):

image

Looking good!
Merging.

@filipefurtad0 filipefurtad0 merged commit 1130bf8 into openfoodfoundation:master May 22, 2024
52 checks passed
@dacook dacook removed the pr-staged-fr staging.coopcircuits.fr label May 22, 2024
@mkllnk mkllnk deleted the enterprise-data-purge branch May 28, 2024 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Clean up unused OFN Enterprise fields
4 participants