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.

Sign up for GitHub

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

Jump to bottom

Filtering Area Based on Currently Edited Seeding Log #628

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Filtering Area Based on Currently Edited Seeding Log #628

wants to merge 6 commits into from

Conversation

FutzMonitor
Copy link
Collaborator

@FutzMonitor FutzMonitor commented Mar 3, 2023 •
edited
Loading

Pull Request Description

Closes #585. This PR aims to remove the user's ability to assign the incorrect area to seeding logs when they're being edited. For example, a direct seeding should not be allowed to be assigned a greenhouse area and a tray seeding should not be allowed to be assigned a field area.

This pull request is NOT ready to be merged.
There are a couple of things that need to be addressed before merging.

  • Conversation about implementation

Licensing Certification

FarmData2 is a Free Cultural Work and all accepted contributions are licensed as described in the LICENSE.md file. This requires that the contributor holds the rights to do so. By submitting this pull request I certify that I satisfy the terms of the Developer Certificate of Origin for its contents.

@FutzMonitor
1. Changes to the seedingReport.html
115e4c8 
- Added a new function to the created hook. This is the 'getAllPages(area)' in order to get the JSON objects for all the areas. I need this data because currently the page only has the names of the areas available to it but not whether the area belongs to the greenhouse or not.
- To the point above, this is incredibly inefficient in my mind. Should we be caching these areas? I imagine they're already cached if someone is visiting the Seeding Input.
- Two new data variables have been added: 'editRowID' which will be assigned the ID of the row being edited which is returned by the CustomTable's edit-clicked event. The other one is the 'editingAreaFilter' which serves as the dynamic area array that is sent to the CustomTable with direct/tray seeding areas depending on what log is being edited. There's also the 'areaJSONList' which is used for holding the JSON objects for all areas.
- A new function has been added 'updateRowEditID'. This name should be changed but the function basically assigns 'editingAreaFilter with the appropriate area names depending on what seeding log is being edited.
- The CustomTable feeds the edit-clicked straight to the new 'updateRowEditID'. I couldn't figure out how to run two functions because doing so wouldn't send the payload to the new function, so I moved the execution of the 'disableFilters' into the new function.
- 'enableFilters' also has an added line that emptys the dynamic list now that it's not needed.
@FutzMonitor
Copy link
Collaborator Author

FutzMonitor commented Mar 3, 2023 •
edited
Loading

There are a couple of issues that I have with my own implementation. However, it will hopefully serve as a starting point of conversation to resolve the overall issue.

  1. I have added a getAllPages call on line 864 which fetches all the area JSON objects and saves them into a new Vue data variable called areaJSONList on line 100. I made this decision because the computed function responsible for filling the CustomTableComponent with the areas only has the area names. It doesn't have any other information (to my knowledge) that can help in distinguishing an area between being part of the greenhouse or being part of the field. Consequently, using the Seeding Input page as example, I decided to get the JSON objects to help me distinguish between the two.
  2. The updateRowEditID (name change pending) on line 156 is incredibly inefficient in my mind. It's not noticeable when I'm testing it but filtering through the lists every single time we want to figure out if the seeding log being edited is a tray or direct seeding is wasteful of resources. A potential alternative that I though of is that after getting our response from the new getAllPages line added to the page, we can filter the areas into 2 new Vue data variables called (something along the lines of) trayAreas and fieldAreas and then the updateRowEditID simply figures out the log is a direct or tray seeding. Afterwards, either a computed or another Vue data variable could be saved with the needed areas and then passed into the computed method for the CustomTableComponent. After editing is done, this intermediary method/variable can be reset (if that's even necessary).
  3. I don't really like that I had to remove the disableFilters call from the event handler inside the CustomTableComponent element tag in the Seeding Report. If someone could point me to some sort of resource that allows me to call both while still being able to pass the event's payload to updateRowEditID that would be much appreciated. I tried following some examples like this one but the event's payload wasn't going through.
  4. There is no attempt to constrain/filter crops in this PR. There are seeds that are always planted directly into the ground and seeds that are always tray seeded. Is there something in the JSON object/some sort of crop information that details this? If not, should there be an issue opened to add this information? It might be worthwhile to amend the original issue (Seeding Report: Filtering Area Name Arrays Depending on Seeding Log #585) to simply deal with area for now and then create a separate one for crop filtering. Additionally, this will generate questions about the Seeding Input page. For example, if someone picks potatoes and potatoes are always direct seedings, then should we disable the radio buttons and only allow direct seeding information to be inputted? This is a conversation we should have with Matt.

@FutzMonitor
1. Changes to seedingReport.html
9826451 
- Minor changes
@FutzMonitor
Merge branch 'main' into FilteringAreaAndCrop
b7cf679
@FutzMonitor
1. Changes to seedingReport.html
762d247 
- Renamed 'updateRowEditID' to 'updateEditable'. The method takes the ID of the emitted 'edit-clicked' event to search the list of logs for the matching log and check its seeding type. If the seeding type is direct: the editableAreas is set to only field areas; if the seeding type is tray seeding: the editableAreas is set only to greenhouse areas.
- Three new arrays have been made: editableAreas is returned in the computed function that returns all the column information to the CustomTableComponent. Its value is assigned in 'updatEditable'. The greenhouseAreas' array is assigned its value on created. The 'fieldAreas' array is assigned its value on created.
- Figured out how to add the 'disableFilters' back in the CustomTable tag while still passing the event payload to the 'updateEditableAreas' method.
- Created now caches araeas and code was added to assign the values for 'greenhouseAreas' and 'fieldAreas'.
- CreatedCounter updated to 9 since although two more created API calls were technically created only one or the other will actually be called.
@FutzMonitor
1. Changes to seedingReport.html
d635a53 
- Removed some commented code
@FutzMonitor FutzMonitor marked this pull request as ready for review March 10, 2023 15:28
@FutzMonitor
1. Changes to seedingReport.html
ac62b83 
- Removed computed method for area names as nothing but the CustomTable was using it and it no longer uses it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Seeding Report: Filtering Area Name Arrays Depending on Seeding Log
1 participant
@FutzMonitor