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

Add Unique iDs for each wins entry #2635

Conversation

lopezpedres
Copy link
Member

Fixes #2385

What changes did you make and why did you make them?

  • Added Unique IDs for each object
  • Refactored App Script Code
  • Tested in my Personal GitHub Repo
Video of applied changes

The main goal of the video is to show how the unique IDs are added to the wins object

trial__wins-data.json.at.main.lopezpedres_trial.-.Google.Chrome.2021-12-27.11-55-55.mp4

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b lopezpedres-add-uniqueID-wins-page-2385 gh-pages
git pull https://github.com/lopezpedres/website.git add-uniqueID-wins-page-2385

@github-actions github-actions bot added Feature: Google Apps Scripts Automation related to Google Apps Scripts Fun Congrats! You finished two good first issues. Please only do one of these P-Feature: Wins Page https://www.hackforla.org/wins/ role: back end/devOps Tasks for back-end developers Complexity: Medium labels Dec 27, 2021
@lopezpedres
Copy link
Member Author

Once this PR gets approved, the actual App Script is going to be modified :)

@SAUMILDHANKAR
Copy link
Member

@lopezpedres Hi Miguel, thanks for working on the issue and adding the video. It looks good. Is line 44 the only change that you made in the App script? Let me know when you are available to go over the changes. We can test if all the sections are still working fine. I don't think we need to merge this. Once done we can just close the issue.

Copy link
Member

@SAUMILDHANKAR SAUMILDHANKAR left a comment

Choose a reason for hiding this comment

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

Please share the code changes that you made in your App Scrip.

@lopezpedres
Copy link
Member Author

@lopezpedres Hi Miguel, thanks for working on the issue and adding the video. It looks good. Is line 44 the only change that you made in the App script? Let me know when you are available to go over the changes. We can test if all the sections are still working fine. I don't think we need to merge this. Once done we can just close the issue.

Hi @SAUMILDHANKAR, yes It's only one line! It was a pretty easy issue. The hard part was to get familiar with App Script and to find the reason why my code wasn't working. We can go over the changes this Tuesday if you are free, maybe in-office hours?

@SAUMILDHANKAR
Copy link
Member

@lopezpedres Thanks Miguel. Lets go over it during the office hours. I wanted to discuss the case where if one of the rows' display value is changed to false, the unique IDs will change. If we can use timestamp as the unique ID instead, this could be resolved since they are more unique.

@lopezpedres
Copy link
Member Author

@lopezpedres Thanks Miguel. Lets go over it during the office hours. I wanted to discuss the case where if one of the rows' display value is changed to false, the unique IDs will change. If we can use timestamp as the unique ID instead, this could be resolved since they are more unique.

@SAUMILDHANKAR That sounds like a good idea! I can start working on it 👍

@SAUMILDHANKAR
Copy link
Member

Review ETA: 2/15/22
Availability: Friday 3-6 PM

@SAUMILDHANKAR
Copy link
Member

SAUMILDHANKAR commented Mar 2, 2022

I am closing this PR as there is no need to merge _wins-data.json file. For this issue, data should get merged through the bot account. The code changes suggested by Miguel are in a comment in the issue. Also, adding ignore label as there is no need of QA on this and all the information is available in the linked issue. Thanks Miguel for working on this. Great job!

@SAUMILDHANKAR SAUMILDHANKAR added the Ignore: Duplicate Issue or pull request is a duplicate label Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Medium Feature: Google Apps Scripts Automation related to Google Apps Scripts Fun Congrats! You finished two good first issues. Please only do one of these Ignore: Duplicate Issue or pull request is a duplicate P-Feature: Wins Page https://www.hackforla.org/wins/ role: back end/devOps Tasks for back-end developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Apps Script: Add unique IDs as object keys for each wins entry
2 participants